On Tue, Dec 03, 2013 at 11:39:15AM +0100, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1033061 > > Currently, initialization of drivers is done in a separate thread. This > is done for several reasons: a driver that is initialized may require > running event loop, it may take ages to initialize driver (e.g. due to > autostarting domains). While the thread is spawn and run, the main() > continues its execution. However, if something goes bad, or the event > loop is just exited (e.g. due to a --timeout or SIGINT) we try to > cleanup all the drivers. So we have two threads running Initialize() and > Cleanup() concurrently. This may result in accessing stale pointers - > e.g. netcf driver will free() itself in stateCleanup callback, while the > init thread may come, open a dummy connection in order to autostart some > domains and voilà: do_open() iterates over interface drivers and > accesses stale netcf driver. > > The fix consists in not running stateCleanup if the init thread is still > running. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > daemon/libvirtd.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c > index aef1546..49c42ad 100644 > --- a/daemon/libvirtd.c > +++ b/daemon/libvirtd.c > @@ -108,6 +108,8 @@ virNetServerProgramPtr remoteProgram = NULL; > virNetServerProgramPtr qemuProgram = NULL; > virNetServerProgramPtr lxcProgram = NULL; > > +volatile bool driversInitialized = false; > + > enum { > VIR_DAEMON_ERR_NONE = 0, > VIR_DAEMON_ERR_PIDFILE, > @@ -912,6 +914,8 @@ static void daemonRunStateInit(void *opaque) > goto cleanup; > } > > + driversInitialized = true; > + > #ifdef HAVE_DBUS > /* Tie the non-priviledged libvirtd to the session/shutdown lifecycle */ > if (!virNetServerIsPrivileged(srv)) { > @@ -1546,7 +1550,8 @@ cleanup: > > daemonConfigFree(config); > > - virStateCleanup(); > + if (driversInitialized) > + virStateCleanup(); Don't we technically need to use an int and atomic int APIs for these changes ? 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list