[libvirt] libvirtd deadlock on shutdown

[Daniel P. Berrange]

On Wed, Jun 20, 2012 at 03:07:16PM -0600, 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().
> 
> Thanks,
> Jim
> 

> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index ae19e84..edd3196 100644
> --- a/src/rpc/virnetserver.c
> +++ 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);
>  
>      for (i = 0 ; i < srv->nsignals ; i++) {
>          sigaction(srv->signals[i]->signum, &srv->signals[i]->oldaction, NULL);

The virNetServerPtr object is ref-counted, so technical;ly
only decrementing the ref count needs to be protected. Once
the ref count hits 0, then only the current thread (and the
workers) should be using the virNetServerPtr instance.

So, yes, it is safe to call virNetServerUnlock before
virThreadPoolFree. Furthermore, it is /not/ required
to call virNetServerLock afterwards - no other thread
should be using it now the workers are dead. So you
can skip that extra lock call, and also remove the
unlock call much later


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 :|