Re: [gtk-vnc-devel] crash in coroutine

[Anthony Liguori <anthony codemonkey ws>]

Daniel P. Berrange wrote:
> On Thu, Jan 17, 2008 at 03:10:47PM -0300, Jonh Wendell wrote:
>   
> > Hi, guys.
> >
> > There is a bug opened against vinagre, about a crash when a host is not
> > found. The bug report is at: https://bugs.launchpad.net/bugs/183169
> >
> > In Ubuntu Gutsy (where I am running now) it works fine. But on Hardy, or
> > even in Foresight VMWare image (running GNOME 2.21 alpha) the bug is
> > reproducible.
> >     
>
> Ok, I think I understand why this happens. In the last bit of code in the
> coroutine before it exits it does
>
>         g_object_unref(G_OBJECT(obj));
>
> In this case it is causing the vncdisplay widget to be destroyed & free'd.
> This includes the co-routine state. Unfortunately, this co-routine state
> is still needed in order to switch back to the main context.
>   

Good analysis.  I find it strange that it only crashes in some 
circumstances.  At any rate, I think I have a simpler solution.  Instead 
of unref'ing the object and the returning to exit the coroutine, we can 
just add an idle function to unref the object which gives enough time 
for the coroutine to exit before getting deleted.  Let me know if the 
attached patch fixes the problem.

Regards,

Anthony Liguori

> Rather than unref'ing the object in vnc_coroutine cleeanup code, we need
> to register a 'release' function when initially creating the coroutine
> and unref it there instead. 
>
> Oh, and we need to fix 'coroutine_swap/_coroutine_release so that they
> don't access the 'struct coroutine' object after the 'release' callback
> has been called.
>
> The same problem will deinitely impact both ucontext & gthread coroutines.
>
> Dan.
>   

diff -r 3a0afe359edf src/vncdisplay.c
--- a/src/vncdisplay.c	Wed Jan 16 16:51:46 2008 -0300
+++ b/src/vncdisplay.c	Fri Jan 18 22:19:46 2008 -0600
@@ -795,6 +795,17 @@ static const struct gvnc_ops vnc_display
 	.render_jpeg = on_render_jpeg,
 };
 
+/* we use an idle function to allow the coroutine to exit before we actually
+ * unref the object since the coroutine's state is part of the object */
+static gboolean delayed_unref_object(gpointer data)
+{
+	VncDisplay *obj = VNC_DISPLAY(data);
+
+	g_assert(obj->priv->coroutine.exited == TRUE);
+	g_object_unref(G_OBJECT(data));
+	return FALSE;
+}
+
 static void *vnc_coroutine(void *opaque)
 {
 	VncDisplay *obj = VNC_DISPLAY(opaque);
@@ -818,7 +829,7 @@ static void *vnc_coroutine(void *opaque)
 	int ret;
 
 	if (priv->gvnc == NULL || gvnc_is_open(priv->gvnc)) {
-		g_object_unref(G_OBJECT(obj));
+		g_idle_add(delayed_unref_object, obj);
 		return NULL;
 	}
 
@@ -875,7 +886,7 @@ static void *vnc_coroutine(void *opaque)
 	g_signal_emit (G_OBJECT (obj),
 		       signals[VNC_DISCONNECTED],
 		       0);
-	g_object_unref(G_OBJECT(obj));
+	g_idle_add(delayed_unref_object, obj);
 	/* Co-routine exits now - the VncDisplay object may no longer exist,
 	   so don't do anything else now unless you like SEGVs */
 	return NULL;