On Wed, Jun 20, 2012 at 03:31:02PM -0600, Eric Blake wrote: > On 06/20/2012 03:07 PM, Jim Fehlig wrote: > > I'm looking into a libvirtd deadlock on daemon shutdown. The deadlock > > occurs when shutting down virNetServer. If the handling of a job is in > > flight in virNetServerHandleJob(), the virNetServer lock is acquired > > when freeing job->prog (src/rpc/virnetserver.c:167). But the lock is > > already held in virNetServerFree(), which is blocked in > > virThreadPoolFree() waiting for all the workers to finish. No progress > > can be made. > > > > The attached hack fixes the problem, but I'm not convinced this is an > > appropriate fix. Is it necessary to hold the virNetServer lock when > > calling virNetServerProgramFree(job->prog)? I notice the lock is not > > held in the error path of virNetServerHandleJob(). > > > > > +++ b/src/rpc/virnetserver.c > > @@ -774,7 +774,9 @@ void virNetServerFree(virNetServerPtr srv) > > for (i = 0 ; i < srv->nservices ; i++) > > virNetServerServiceToggle(srv->services[i], false); > > > > + virNetServerUnlock(srv); > > virThreadPoolFree(srv->workers); > > + virNetServerLock(srv); > > As written, this can't be right, because it reads a field of srv outside > the locks. But maybe you meant: > > <type> tmp = srv->workers; > srv->workers = NULL; > virNetServerUnlock(srv); > virThreadPoolFree(tmp); > virNetServerLock(srv); > > as a possible fix that doesn't violate the safety rules of reading > fields from srv outside a lock. While you are technically correct in the most general sense, there is not actually any harm in dereferencing srv->workers here, since now the ref count is 0, only the current thread & the workers are allowed to be accessing 'srv', an the workers don't touch the 'srv->workers' field at all. 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