[libvirt] [PATCH] rpc: Avoid deadlock when closing client connection

[Daniel P. Berrange]

On Mon, Feb 18, 2013 at 03:52:40PM +0100, Jiri Denemark wrote:
> We need to drop the server lock before calling virObjectUnlock(client)
> since in case we had the last reference to the client, its dispose
> callback would be called and that could possibly try to lock the server
> and cause a deadlock. This is exactly what happens when there is only
> one QEMU domain running and it is marked to be autodestroyed when the
> connection dies. This results in qemuProcessAutoDestroy ->
> qemuProcessStop -> virNetServerRemoveShutdownInhibition call sequence,
> where the last function locks the server.
> ---
>  src/rpc/virnetserver.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index 95333d0..e536cc3 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -1120,7 +1120,7 @@ void virNetServerRun(virNetServerPtr srv)
>              if (virNetServerClientWantClose(srv->clients[i]))
>                  virNetServerClientClose(srv->clients[i]);
>              if (virNetServerClientIsClosed(srv->clients[i])) {
> -                virObjectUnref(srv->clients[i]);
> +                virNetServerClientPtr client = srv->clients[i];
>                  if (srv->nclients > 1) {
>                      memmove(srv->clients + i,
>                              srv->clients + i + 1,
> @@ -1131,6 +1131,10 @@ void virNetServerRun(virNetServerPtr srv)
>                      srv->nclients = 0;
>                  }
>  
> +                virObjectUnlock(srv);
> +                virObjectUnref(client);
> +                virObjectLock(srv);
> +
>                  goto reprocess;
>              }
>          }

Ordinarily unlocking in the middle of aloop is unsafe, but the
'goto reprocess' restarts the entire loop, so it is fine in this
scenario.

ACK


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