[gjs] closure: Clear closure functions in idle handler

[Philip Chimento <pchimento src gnome org>]

commit 334ba969a9e0cae39300dd6bcf1603486c92ac45
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Feb 5 21:10:48 2017 -0800

    closure: Clear closure functions in idle handler
    
    When closures are invalidated, they need to drop the reference to their
    JS functions. However, it appears that closure invalidate notifications
    can be sent out during garbage collection. Incremental GC does not allow
    changing what you trace while GC is going on, so we have to keep tracing
    the JS functions, and only drop them in an idle handler later on.
    
    We still decrement the memory counter in the invalidate notification,
    though, because if you are doing a memory counter check at the end of
    your program, the idle handler may not have run yet.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776966

 gi/closure.cpp |   32 +++++++++++++++++++++++---------
 gi/object.cpp  |   18 +++++++++++++++---
 2 files changed, 38 insertions(+), 12 deletions(-)
---
diff --git a/gi/closure.cpp b/gi/closure.cpp
index 86a13e3..1f613a7 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -118,6 +118,25 @@ global_context_finalized(JS::HandleObject obj,
     }
 }
 
+/* Closures have to drop their references to their JS functions in an idle
+ * handler, because otherwise the closure might stop tracing the function object
+ * in the middle of garbage collection. That is not allowed with incremental GC.
+ */
+static gboolean
+closure_clear_idle(void *data)
+{
+    auto closure = static_cast<GjsClosure *>(data);
+    gjs_debug_closure("Clearing closure %p which calls object %p",
+                      &closure->priv, closure->priv.object.get());
+
+    closure->priv.obj.reset();
+    closure->priv.context = nullptr;
+    closure->priv.runtime = nullptr;
+
+    g_closure_unref(static_cast<GClosure *>(data));
+    return G_SOURCE_REMOVE;
+}
+
 /* Invalidation is like "dispose" - it is guaranteed to happen at
  * finalize, but may happen before finalize. Normally, g_closure_invalidate()
  * is called when the "target" of the closure becomes invalid, so that the
@@ -158,22 +177,17 @@ closure_invalidated(gpointer data,
                       "removing our destroy notifier on global object)",
                       closure);
 
-    c->obj.reset();
-    c->context = NULL;
-    c->runtime = NULL;
+    g_idle_add(closure_clear_idle, closure);
+    g_closure_ref(closure);
 }
 
 static void
 closure_set_invalid(gpointer  data,
                     GClosure *closure)
 {
-    Closure *self = &((GjsClosure*) closure)->priv;
-
-    self->obj.reset();
-    self->context = NULL;
-    self->runtime = NULL;
-
     GJS_DEC_COUNTER(closure);
+    g_idle_add(closure_clear_idle, closure);
+    g_closure_ref(closure);
 }
 
 static void
diff --git a/gi/object.cpp b/gi/object.cpp
index 8b63651..776ca4d 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -1675,15 +1675,27 @@ gjs_lookup_object_prototype(JSContext *context,
     return proto;
 }
 
-static void
-signal_connection_invalidated (gpointer  user_data,
-                               GClosure *closure)
+/* Removing the signal connection data from the list means that the object stops
+ * tracing the JS function objects belonging to the closures. Incremental GC
+ * does not allow that in the middle of a garbage collection. Therefore, we must
+ * do it in an idle handler.
+ */
+static gboolean
+signal_connection_invalidate_idle(void *user_data)
 {
     ConnectData *connect_data = (ConnectData *) user_data;
 
     connect_data->obj->signals = g_list_delete_link(connect_data->obj->signals,
                                                     connect_data->link);
     g_slice_free(ConnectData, connect_data);
+    return G_SOURCE_REMOVE;
+}
+
+static void
+signal_connection_invalidated(void     *data,
+                              GClosure *closure)
+{
+    g_idle_add(signal_connection_invalidate_idle, data);
 }
 
 static bool