[Libguestfs] [PATCH v3 3/5] threads: Use thread-local storage for errors.

[Daniel P. Berrange]

On Mon, Jul 10, 2017 at 01:48:13PM +0200, Pino Toscano wrote:
> On Tuesday, 27 June 2017 13:55:57 CEST Richard W.M. Jones wrote:
> > We permit the following constructs in libguestfs code:
> > 
> >   if (guestfs_some_call (g) == -1) {
> >     fprintf (stderr, "failed: error is %s\n", guestfs_last_error (g));
> >   }
> > 
> > and:
> > 
> >   guestfs_push_error_handler (g, NULL, NULL);
> >   guestfs_some_call (g);
> >   guestfs_pop_error_handler (g);
> > 
> > Neither of these would be safe if we allowed the handle to be used
> > from threads concurrently, since the error string or error handler
> > could be changed by another thread.
> > 
> > Solve this in approximately the same way that libvirt does: by making
> > the error, current error handler, and stack of error handlers use
> > thread-local storage (TLS).
> > 
> > The implementation is not entirely straightforward, mainly because
> > POSIX doesn't give us useful destructor behaviour, so effectively we
> > end up creating our own destructor using a linked list.
> 
> I'm not sure which behaviour you are referring to, but it should work
> just fine -- in the destructor function, cast the void* argument to the
> error_data struct, and free the linked list associated.
> 
> The only problem is that the tls gnulib implementation on Windows
> (actually, on mingw) makes no use for the destructor function, but
> a) we don't have a working Windows port ATM (not even on other
>    non-Linux Unices)
> b) using pthread or pth on mingw would solve this

For libvirt we decided to exclusively support pthread (thus requiring use
of the mingw winpthreads  package for Win32.

> I'd just use the destructor function approach, so there is less
> complexity involved, and the per-thread resources are properly free'd
> on thread exit (and not piling up until the handle destruction).

There is one critical gotcha with destructors and libraries - if your
library can be loaded via 'dlopen' (eg because its imported from a
language binding) you must guarantee that is resident in memory
*forever*. If you don't do this, then something can dlclose() you,
unloading the code that contains the destructor. You'll then crash
when a thread exits trying to invoke the destructor that is no longer
resident.

The workaround for this is to link the library with '-z nodelete',
which prevents you ever being unloaded from memory, even after a
dlclose(). We did this in libvirt in

  commit 8e44e5593eb9b89fbc0b54fde15f130707a0d81e
  Author: Daniel P. Berrange <berrange at redhat.com>
  Date:   Thu Sep 1 17:57:06 2011 +0100

    Prevent crash from dlclose() of libvirt.so


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|