[gjs/wip/ptomato/785657: 5/6] object: invalidate signal connection later

[Philip Chimento <pchimento src gnome org>]

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

    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.

 gi/object.cpp  |   42 +++++++++++++++++++++++++++++++++---------
 gi/object.h    |    1 +
 gjs/engine.cpp |    5 +++++
 3 files changed, 39 insertions(+), 9 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 260e288..63306e2 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -84,6 +84,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);
@@ -1268,18 +1270,33 @@ 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. */
 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 */
-        GClosure *closure = *priv->signals.begin();
-        g_closure_invalidate(closure);
-        /* Erase element if not already erased */
-        priv->signals.erase(closure);
+    for (GClosure *closure : priv->signals) {
+        bool inserted;
+        std::tie(std::ignore, inserted) = closures_to_invalidate.insert(closure);
+        if (inserted)
+            g_closure_ref(closure);
     }
 }
 
@@ -1480,6 +1497,13 @@ object_instance_finalize(JSFreeOp  *fop,
      * an instance's GObject is already freed at this point. */
     invalidate_all_signals(priv);
 
+    /* The invalidate notifier will want to use this object, so remove it */
+    for (GClosure *closure : priv->signals) {
+        g_closure_remove_invalidate_notifier(closure, priv,
+                                             signal_connection_invalidated);
+    }
+    priv->signals.clear();
+
     /* Object is instance, not prototype, AND GObject is not already freed */
     if (priv->gobj) {
         bool had_toggle_up;
diff --git a/gi/object.h b/gi/object.h
index 299c848..b1c037e 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -58,6 +58,7 @@ bool      gjs_typecheck_is_object(JSContext       *context,
 
 void gjs_object_prepare_shutdown(void);
 void gjs_object_clear_toggles(void);
+void gjs_object_invalidate_closures(void);
 
 void gjs_object_define_static_methods(JSContext       *context,
                                       JS::HandleObject constructor,
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