Re: [Qemu-devel] [PATCH] char: do not use atexit cleanup handler

[Daniel P. Berrange]

On Mon, Jul 04, 2016 at 06:43:42PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Jul 4, 2016 at 6:31 PM, Daniel P. Berrange <address@hidden> wrote:
> > On Mon, Jul 04, 2016 at 05:38:23PM +0200, address@hidden wrote:
> >> From: Marc-André Lureau <address@hidden>
> >>
> >> It turns out qemu is calling exit() in various places from various
> >> threads without taking much care of resources state. The atexit()
> >> cleanup handlers cannot easily destroy resources that are in use (by
> >> the same thread or other).
> >
> > [snip]
> >
> >> Instead of using a atexit() handler, only run the chardev cleanup as
> >> initially proposed at the end of main(), where there are less chances
> >> (hic) of conflicts or other races.
> >
> > This doesn't really seem all that much safer. There's still plenty of
> > chance that threads are running in the background at the end of the
> > main() method, so plenty of scope for the qemu_chr_cleanup() call to
> > cause threads to segv by destroying the chardevs they're using behind
> > their back.
> 
> Yep yep
> 
> Note, just before the end of main() all CPU threads should be frozen.
> 
> Is there any reason why there is no support for joining an cleaning up
> all the running threads at this point?
> 
> > IIUC, the original intent here was that we call unlink() on the UNIX
> > socket paths when QEMU exits.
> >
> > Surely we can come up with a way to that, and only that, upon exit,
> > without actually having to free the chardev memory with all the risks
> > that entails.
> 
> Doesn't sound that simple if you can't take the lock. And when you do
> you can't destroy it. It sounds hard to avoid races, so I might be
> missing something.

The other option is to not try to do this at the chardev level at all.
Instead we could have the QIOChannelSocket impl maintain a list of UNIX
socket paths that need unlinking, and have it register an atexit() handler
itself, whose sole purpose is to unlink the paths. We could simply have a
global GList of char* UNIX socket paths, so the atexit handler would not
touch the QIOChannelSocket objects at all.  This avoiding all races or
issues with concurrent access.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|