[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Deadlock when using custom handlers



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 sigxcpu org>
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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]