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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list