On 8/25/20 5:00 AM, Richard W.M. Jones wrote:
+=head2 C<.default_export> + + const char *default_export (int readonly, int is_tls);
Oh fun. For some plugins (like ondemand), this is trivial: return a compile-time constant string. But for others (like sh and eval), there's a lifetime issue: this callback is used _before_ .open, ergo there is no handle struct that it can be associated with. What's more, this is called _after_ .preconnect, which means it is logical to expect that the default export name might change over time (consider a plugin that advertises the largest file in a directory as its default, but where the directory can change _which_ file is largest between when the first client connects and when the second client connects). And the string returned by the sh script is in malloc'd memory (by it's very nature of coming from the user script, rather than being a compile-time constant). Without a handle to store this string in, we would have a memory leak: there is no way to associate this inside the handle's struct so that .close can reclaim it, but storing it globally is not thread-safe to parallel client connections.
I'm not sure there's an actual problem here, because there is still a connection object inside the server any time we have a TCP connection, so you can just store it there, unless I'm misunderstanding something.
Plugins cannot access the TCP connection except through public APIs ;) I've got the patch working (it passes 'make check-valgrind'), so I'll post it later this morning to demo why it is useful.
But anyway ...
So I'm thinking I need to add a helper function:
const char *nbdkit_string_intern (const char *str);
Return a pointer to a copy of str, where nbdkit owns the lifetime of the copy (allowing the caller to not have to worry about persisting str indefinitely). If called when there is no client connection (such as during .load), the copy will remain valid through .unload; if called in the context of a client connection (any callback from .preconnect through .close), the copy will remain valid through .close.
I'm a bit unclear why the plugin would have to call this (or this function be in the public API at all). Why can't string interning be done inside the server. Have a global struct where strings returned from the plugin are interned, and free it on server exit.
The server IS doing the interning, and associating the intern'd strings with the connection or with overall server exit. But plugins need to use it when they need a longer lifetime but don't want to clean up themselves. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org