Re: [libvirt] libvirtd deadlock on shutdown

On Thu, Jun 21, 2012 at 09:56:54AM -0600, Jim Fehlig wrote: > From 583be33213e922899b23f036494886397b2549dc Mon Sep 17 00:00:00 2001 > From: Jim Fehlig <jfehlig(a)suse.com&gt; > Date: Thu, 21 Jun 2012 09:21:44 -0600 > Subject: [PATCH] Fix deadlock on libvirtd shutdown > > When shutting down libvirtd, the virNetServer shutdown can deadlock > if there are in-flight jobs being handled by virNetServerHandleJob(). > virNetServerFree() will acquire the virNetServer lock and call > virThreadPoolFree() to terminate the workers, waiting for the workers > to finish. But in-flight workers will attempt to acquire the > virNetServer lock, resulting in deadlock. > > Fix the deadlock by unlocking the virNetServer lock before calling > virThreadPoolFree(). This is safe since the virNetServerPtr object > is ref-counted and only decrementing the ref count needs to be > protected. Additionally, there is no need to re-acquire the lock > after virThreadPoolFree() completes as all the workers have > terminated. > --- > src/rpc/virnetserver.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c > index ae19e84..9d71e53 100644 > --- a/src/rpc/virnetserver.c > +++ b/src/rpc/virnetserver.c > @@ -766,10 +766,9 @@ void virNetServerFree(virNetServerPtr srv) > virNetServerLock(srv); > VIR_DEBUG("srv=%p refs=%d", srv, srv->refs); > srv->refs--; > - if (srv->refs > 0) { > - virNetServerUnlock(srv); > + virNetServerUnlock(srv); > + if (srv->refs > 0) > return; > - } > > for (i = 0 ; i < srv->nservices ; i++) > virNetServerServiceToggle(srv->services[i], false); > @@ -805,7 +804,6 @@ void virNetServerFree(virNetServerPtr srv) > virNetServerMDNSFree(srv->mdns); > #endif > > - virNetServerUnlock(srv); > virMutexDestroy(&srv->lock); > VIR_FREE(srv); > } > ACK, We really should resurrect the patches adding virAtomic for refcounting :-)