Libguestfs - May 2022 - nbdkit worker thread shutdown

Hello,

When nbdkit calls a plugin's unload() method, is it guaranteed that all
pending requests have been handled (and all worker threads exited)?

(I would have expected this to be the case, but I'm getting errors from
threads accessing data that my unload() handler frees, so I wanted to
confirm my assumption).


Best,
-Nikolaus

-- 
GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             ?Time flies like an arrow, fruit flies like a Banana.?

On 05/05/22 09:59, Nikolaus Rath wrote:
> Hello,
> 
> When nbdkit calls a plugin's unload() method, is it guaranteed that all
> pending requests have been handled (and all worker threads exited)?
> 
> (I would have expected this to be the case, but I'm getting errors from
> threads accessing data that my unload() handler frees, so I wanted to
> confirm my assumption).
>From <https://libguestfs.org/nbdkit-plugin.3.html>:
.cleanup

    is called once after all connections have been closed, prior to
    unloading the plugin from memory. This is only called if .after_fork
    succeeded earlier (even in cases where nbdkit did not fork but is
    running in the foreground), which makes it a good place to
    gracefully end any background threads.

So I would think you need to either join all threads in .cleanup (at the
latest), waiting for them to drain any pending work, or cancel them
(with appropriate cancellation handlers in the background threads).

... I'm saying "at the latest" because in the vddk plugin, for
example,
pthread_join is called earlier: in .close. I guess that depends on
whether your background thread(s) work at the connection level, or
higher than that (at plugin level).

Thanks
Laszlo

On Thu, May 05, 2022 at 08:59:56AM +0100, Nikolaus Rath
wrote:
> Hello,
> 
> When nbdkit calls a plugin's unload() method, is it guaranteed that all
> pending requests have been handled (and all worker threads exited)?
> 
> (I would have expected this to be the case, but I'm getting errors from
> threads accessing data that my unload() handler frees, so I wanted to
> confirm my assumption).
.unload is literally called just before dlclose (see
server/backend.c:backend_unload) so if a thread in nbdkit was to
access something in your plugin after .unload then that would be quite
a serious bug in nbdkit.  It would be bound to crash because the code
and data has been unloaded from the process.  This implies too that
all connections would have been closed already.

Normally the server has no view into your plugin except through the
pointers in the plugin structure, and it shouldn't be using those
after .unload.  Can you give us some clue as to what exactly nbdkit is
accessing in your plugin?

The difference between .cleanup and .unload is the global ordering of
plugins/filters.  .cleanup is called first, when the plugin and
filters are still in memory.  .unload is then called on each filter &
the plugin (IIRC, plugin first), and so other things may have already
been unloaded from memory.  You almost certainly don't need to worry
about this difference -- almost everything can use .unload -- but it
matters in some corner cases (see commit ce9e6066354 for details).

If your plugin needs to create its own background threads, it should
create them in .after_fork (or .open if per-connection), and can clean
them up in .unload (or .close).  VDDK plugin is a mature example of
per-connection background threads.  Torrent plugin has a global
background thread that it (incorrectly) doesn't clean up.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org