[Spice-devel] [spice-gtk 2/3] coroutine: Track idle notification/emission in struct coroutine

[Christophe Fergeau]

This allows to catch situations when we try to reinit the coroutine
using coroutine_init() while a signal/notify idle is pending.

This can actually happen with cancelled migration and connect_delayed
(which reinits the current coroutine).
If channel_disconnect is called from coroutine context when state is
SPICE_CHANNEL_STATE_SWITCHING:
    - channel_connect() is called and queues an idle which will call
      connect_delayed()
    - then spice_session_set_migration_state() is called and will call
      g_coroutine_object_notify(), which will queue an idle to emit the
      notification, and yield to the main context
    - connect_delayed() gets called in an idle, resets the current
      coroutine, and then yield back to the coroutine context
    - g_coroutine_object_notify() resumes and is confused as the
      notification did not happen.

This can be triggered by adding a g_usleep(10000000); at the beginning
of spice_session_start_migrating(), and by migrating an oVirt VM after
connecting to it.

This is based on a patch suggestion from Marc-André.
---
 gtk/coroutine.h          |  1 +
 gtk/coroutine_ucontext.c |  5 +++++
 gtk/gio-coroutine.c      | 16 ++++++++++++----
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/gtk/coroutine.h b/gtk/coroutine.h
index 78dc467..5525fd3 100644
--- a/gtk/coroutine.h
+++ b/gtk/coroutine.h
@@ -53,6 +53,7 @@ struct coroutine
 	GThread *thread;
 	gboolean runnable;
 #endif
+	guint idle_id;
 };
 
 void coroutine_init(struct coroutine *co);
diff --git a/gtk/coroutine_ucontext.c b/gtk/coroutine_ucontext.c
index d709a33..6580fd7 100644
--- a/gtk/coroutine_ucontext.c
+++ b/gtk/coroutine_ucontext.c
@@ -65,6 +65,11 @@ static void coroutine_trampoline(struct continuation *cc)
 
 void coroutine_init(struct coroutine *co)
 {
+	if (co->idle_id != 0) {
+		g_warn_if_reached();
+		g_source_remove(co->idle_id);
+		co->idle_id = 0;
+	}
 	if (co->stack_size == 0)
 		co->stack_size = 16 << 20;
 
diff --git a/gtk/gio-coroutine.c b/gtk/gio-coroutine.c
index 1458d05..7527138 100644
--- a/gtk/gio-coroutine.c
+++ b/gtk/gio-coroutine.c
@@ -197,6 +197,7 @@ static gboolean emit_main_context(gpointer opaque)
 {
     struct signal_data *signal = opaque;
 
+    g_warn_if_fail(signal->caller->idle_id != 0);
     g_signal_emit_valist(signal->instance, signal->signal_id,
                          signal->detail, signal->var_args);
     signal->notified = TRUE;
@@ -207,12 +208,14 @@ static gboolean emit_main_context(gpointer opaque)
 }
 
 static void
-run_in_idle(GSourceFunc idle_callback, gpointer user_data)
+run_in_idle(GSourceFunc idle_callback, gpointer opaque)
 {
-    struct signal_data *data = (struct signal_data *)user_data;
+    struct signal_data *data = opaque;
+
+    g_warn_if_fail(data->caller->idle_id == 0);
 
     g_object_ref(data->instance);
-    g_idle_add(idle_callback, data);
+    data->caller->idle_id = g_idle_add(idle_callback, data);
     /* This switches to the system coroutine context, lets
      * the idle function run to dispatch the signal, and
      * finally returns once complete. ie this is synchronous
@@ -220,7 +223,11 @@ run_in_idle(GSourceFunc idle_callback, gpointer user_data)
      * an idle function involved
      */
     coroutine_yield(NULL);
-    g_warn_if_fail(data->notified);
+    if (!data->notified) {
+        g_warn_if_reached();
+        g_source_remove(data->caller->idle_id);
+    }
+    data->caller->idle_id = 0;
     g_object_unref(data->instance);
 }
 
@@ -251,6 +258,7 @@ static gboolean notify_main_context(gpointer opaque)
 {
     struct signal_data *signal = opaque;
 
+    g_warn_if_fail(signal->caller->idle_id != 0);
     g_object_notify(signal->instance, signal->propname);
     signal->notified = TRUE;
 
-- 
2.1.0