[libvirt] Deadlock when using custom handlers

[Daniel P. Berrange]

On Fri, Aug 12, 2011 at 11:54:28PM +0200, Guido Günther wrote:
> Hi,
> virt-viewer is using it's own virEventRegisterImpl. With current libvirt
> this can deadlock when connection to nonexistant URIs like
> 
>     qemu+ssh:///unknownhost.example.com/system
> Gdb displays the deadlock nicely:
> 
> #0  0xb7710424 in __kernel_vsyscall ()
> #1  0xb692cf02 in __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/i386/i686/../i486/lowlevellock.S:142
> #2  0xb692839b in _L_lock_728 () from /lib/i386-linux-gnu/i686/cmov/libpthread.so.0
> #3  0xb69281c1 in __pthread_mutex_lock (mutex=0x95e1c30) at pthread_mutex_lock.c:61
> #4  0xb698fd07 in virMutexLock (m=0x95e1c30) at util/threads-pthread.c:85
> #5  0xb6a5dd56 in virNetSocketEventFree (opaque=0x95e1c30) at rpc/virnetsocket.c:1147
> #6  0x080514a1 in virt_viewer_events_remove_handle (watch=1) at virt-viewer-events.c:178
> #7  0xb697264e in virEventRemoveHandle (watch=1) at util/event.c:84
> #8  0xb6a608ed in virNetSocketRemoveIOCallback (sock=0x95e1c30) at rpc/virnetsocket.c:1221
> #9  0xb6a57114 in virNetClientClose (client=0xb5b7e008) at rpc/virnetclient.c:280
> #10 0xb6a46fc4 in doRemoteOpen (conn=0x95e1aa8, priv=0x95b1370, auth=0xbfc3b178, flags=1) at remote/remote_driver.c:705
> #11 0xb6a49612 in remoteOpen (conn=0x95e1aa8, auth=0xbfc3b178, flags=1) at remote/remote_driver.c:820
> #12 0xb69f70c6 in do_open (name=0x95aac10 "qemu+ssh://pustekiste:2222/system", auth=0xbfc3b178, flags=1) at libvirt.c:1054
> #13 0xb69f9b88 in virConnectOpenAuth (name=0x95aac10 "qemu+ssh://pustekiste:2222/system", auth=0xbfc3b178, flags=1) at libvirt.c:1280
> #14 0x080500cf in virt_viewer_start (app=0x95a1010) at virt-viewer.c:483
> #15 0x08053be8 in virt_viewer_app_start (self=0x95a1010) at virt-viewer-app.c:1044
> #16 0x0804f54d in main (argc=1, argv=0xbfc3b3f4) at virt-viewer-main.c:120
> 
> which is the sock->lock:
> 
>     virNetSocketRemoveIOCallback holds sock->lock
> 	-> virEventRemoveHandle
> 	    -> impl_remove_handle
> 		-> opaque->ff
> 		    -> virNetSocketEventFree want to hold sock->lock

In the default libvirt event loop, the 'ff' callback is always invoked
from a "clean" stack in the event loop, so you never have this problem
with re-entrancy.

> Working around this by removing the locks from
> virNetSocketRemoveIOCallback leads to another deadlock:

Yeah this is not a viable approach. 

> 
> I didn't see a simple way to fix this but would welcome any suggestions.

IMHO we just have to document that event loop implementations
should make sure that the 'ff' callbacks are always invoked
from a clean stack. In the case of virt-viewer, this means
changing it to register a g_idle  callback function to invoke
the 'ff' callback.

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