From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44787) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bK7QW-0001RU-Gy for qemu-devel@nongnu.org; Mon, 04 Jul 2016 13:12:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bK7QS-0006yX-7y for qemu-devel@nongnu.org; Mon, 04 Jul 2016 13:12:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58176) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bK7QR-0006yT-W1 for qemu-devel@nongnu.org; Mon, 04 Jul 2016 13:12:52 -0400 Date: Mon, 4 Jul 2016 18:12:48 +0100 From: "Daniel P. Berrange" Message-ID: <20160704171248.GP3763@redhat.com> Reply-To: "Daniel P. Berrange" References: <20160704153823.16879-1-marcandre.lureau@redhat.com> <20160704163139.GM3763@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] char: do not use atexit cleanup handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Cc: Paolo Bonzini , QEMU On Mon, Jul 04, 2016 at 06:43:42PM +0200, Marc-Andr=C3=A9 Lureau wrote: > Hi >=20 > On Mon, Jul 4, 2016 at 6:31 PM, Daniel P. Berrange wrote: > > On Mon, Jul 04, 2016 at 05:38:23PM +0200, marcandre.lureau@redhat.com= wrote: > >> From: Marc-Andr=C3=A9 Lureau > >> > >> 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 chance= s > >> (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. >=20 > Yep yep >=20 > Note, just before the end of main() all CPU threads should be frozen. >=20 > Is there any reason why there is no support for joining an cleaning up > all the running threads at this point? >=20 > > 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. >=20 > 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() handle= r 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 --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|