[gjs/wip/ptomato/785657] WIP - object: invalidate signal connection later

[Philip Chimento <pchimento src gnome org>]

commit 2039b2b214395aaec2f0b30cb30eaa744e54816c
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Aug 20 22:07:27 2017 -0700

    WIP - object: invalidate signal connection later
    
    This defers the call of g_closure_invalidate() until the end of a garbage
    collection. We can't stop tracing signal connections in the middle of
    garbage collection, and we can't defer the actual freeing of the
    closures because that caused too many race conditions with the garbage
    collector. Instead, defer the invalidation.
    
    This probably means that more JS objects will survive a garbage
    collection when they didn't have to, but they should get collected in the
    following garbage collection.
    
    FIXME: Crashes when tearing down objects when the context is destroyed

 gi/object.cpp  |   34 +++++++++++++++++++++++++---------
 gi/object.h    |    2 ++
 gjs/engine.cpp |    5 +++++
 3 files changed, 32 insertions(+), 9 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 631d3ec..4196614 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -91,6 +91,8 @@ extern struct JSClass gjs_object_instance_class;
 
 static std::set<ObjectInstance *> dissociate_list;
 
+static std::set<GClosure *> closures_to_invalidate;
+
 GJS_DEFINE_PRIV_FROM_JS(ObjectInstance, gjs_object_instance_class)
 
 static void            disassociate_js_gobject (GObject *gobj);
@@ -1275,18 +1277,32 @@ associate_js_gobject (JSContext       *context,
     g_object_add_toggle_ref(gobj, wrapped_gobj_toggle_notify, NULL);
 }
 
+/**
+ * gjs_object_invalidate_closures:
+ *
+ * Should be called periodically to clean up all closures that need to be
+ * invalidated as a result of running a garbage collection. Currently we call
+ * it at the end of every GC, in engine.cpp.
+ */
+void
+gjs_object_invalidate_closures(void)
+{
+    for (GClosure *closure : closures_to_invalidate) {
+        g_closure_invalidate(closure);
+        g_closure_unref(closure);
+    }
+    closures_to_invalidate.clear();
+}
+
+/* Called at the end of an object's lifetime, when all signals get disconnected.
+ * Frees the connect data immediately instead of waiting for the closures to
+ * be invalidated. */
 static void
 invalidate_all_signals(ObjectInstance *priv)
 {
-    /* Can't loop directly through the items, since invalidating an item's
-     * closure might have the effect of removing the item from the set in the
-     * invalidate notifier */
-    while (!priv->signals.empty()) {
-        /* This will also free cd, through the closure invalidation mechanism */
-        ConnectData *cd = *priv->signals.begin();
-        g_closure_invalidate(cd->closure);
-        /* Erase element if not already erased */
-        priv->signals.erase(cd);
+    for (ConnectData *cd : priv->signals) {
+        closures_to_invalidate.insert(cd->closure);
+        g_closure_ref(cd->closure);
     }
 }
 
diff --git a/gi/object.h b/gi/object.h
index 69017ef..722fea1 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -60,6 +60,8 @@ void      gjs_object_prepare_shutdown   (JSContext     *context);
 
 void gjs_object_clear_toggles(void);
 
+void gjs_object_invalidate_closures(void);
+
 void gjs_object_define_static_methods(JSContext       *context,
                                       JS::HandleObject constructor,
                                       GType            gtype,
diff --git a/gjs/engine.cpp b/gjs/engine.cpp
index a6d5f48..1da82c6 100644
--- a/gjs/engine.cpp
+++ b/gjs/engine.cpp
@@ -207,6 +207,11 @@ on_garbage_collect(JSContext *cx,
      * garbage collected. */
     if (status == JSGC_BEGIN)
         gjs_object_clear_toggles();
+    /* After garbage collection, invalidate any closures that have been
+     * disconnected. We can't do this during garbage collection because it's
+     * illegal to stop tracing a closure in the middle of GC. */
+    else if (status == JSGC_END)
+        gjs_object_invalidate_closures();
 }
 
 static bool