Hi Daniel, On Sat, Aug 13, 2011 at 08:57:45PM -0700, Daniel P. Berrange wrote: > On Fri, Aug 12, 2011 at 11:54:28PM +0200, Guido Günther wrote: [..snip..] > 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. Sure. This was only to see what else fails. > > > > 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. Patch for virt-viewer attached. I'll come up with a doc patch for libvirt once I have a bit more time. Cheers -- Guido
>From 8bee900e9eb15aeca023d548632a3032d1e9f552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= <agx@xxxxxxxxxxx> Date: Tue, 16 Aug 2011 13:59:53 +0200 Subject: [PATCH] ff callbacks must be invoked from a clean stack http://www.redhat.com/archives/libvir-list/2011-August/msg00554.html --- src/virt-viewer-events.c | 47 ++++++++++++++++++++++++++++++++++++++------- 1 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c index f526fa5..5fc6efb 100644 --- a/src/virt-viewer-events.c +++ b/src/virt-viewer-events.c @@ -159,6 +159,23 @@ virt_viewer_events_update_handle(int watch, } } + +static gboolean +virt_viewer_events_cleanup_handle(gpointer user_data) +{ + struct virt_viewer_events_handle *data = user_data; + + DEBUG_LOG("Cleanup of handle %p", data); + g_return_val_if_fail(data != NULL, FALSE); + + if (data->ff) + (data->ff)(data->opaque); + + free(data); + return FALSE; +} + + static int virt_viewer_events_remove_handle(int watch) { @@ -171,13 +188,14 @@ virt_viewer_events_remove_handle(int watch) DEBUG_LOG("Remove handle %d %d", watch, data->fd); + if (!data->source) + return -1; + g_source_remove(data->source); data->source = 0; data->events = 0; - if (data->ff) - (data->ff)(data->opaque); - free(data); + g_idle_add(virt_viewer_events_cleanup_handle, data); return 0; } @@ -278,6 +296,23 @@ virt_viewer_events_update_timeout(int timer, } } + +static gboolean +virt_viewer_events_cleanup_timeout(gpointer user_data) +{ + struct virt_viewer_events_timeout *data = user_data; + + DEBUG_LOG("Cleanup of timeout %p", data); + g_return_val_if_fail(data != NULL, FALSE); + + if (data->ff) + (data->ff)(data->opaque); + + free(data); + return FALSE; +} + + static int virt_viewer_events_remove_timeout(int timer) { @@ -296,11 +331,7 @@ virt_viewer_events_remove_timeout(int timer) g_source_remove(data->source); data->source = 0; - if (data->ff) - (data->ff)(data->opaque); - - free(data); - + g_idle_add(virt_viewer_events_cleanup_timeout, data); return 0; } -- 1.7.5.4
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list