[gjs/wip/ptomato/mozjs38: 4/23] WIP - get rid of keep-alive

[Philip Chimento <pchimento src gnome org>]

commit d0654455ab689f524ef71995c4993e0b255e1e4f
Author: Philip Chimento <philip endlessm com>
Date:   Wed Feb 1 18:29:11 2017 -0800

    WIP - get rid of keep-alive

 gi/object.cpp         |   97 +++++++++++++++++++++++--------------------------
 gi/object.h           |    2 +
 gjs/context.cpp       |    2 +
 gjs/jsapi-util-root.h |    2 +
 4 files changed, 52 insertions(+), 51 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 0248d46..e17f5ff 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -26,6 +26,7 @@
 #include <memory>
 #include <stack>
 #include <string.h>
+#include <set>
 #include <vector>
 
 #include "object.h"
@@ -51,10 +52,10 @@
 #include <util/hash-x32.h>
 #include <girepository.h>
 
-typedef struct {
+struct ObjectInstance {
     GIObjectInfo *info;
     GObject *gobj; /* NULL if we are the prototype and not an instance */
-    JSObject *keep_alive; /* NULL if we are not added to it */
+    GjsMaybeOwned<JSObject *> keep_alive;
     GType gtype;
 
     /* a list of all signal connections, used when tracing */
@@ -63,7 +64,7 @@ typedef struct {
     /* the GObjectClass wrapped by this JS Object (only used for
        prototypes) */
     GTypeClass *klass;
-} ObjectInstance;
+};
 
 typedef struct {
     ObjectInstance *obj;
@@ -95,6 +96,7 @@ static GHashTable *class_init_properties;
 extern struct JSClass gjs_object_instance_class;
 static GThread *gjs_eval_thread;
 static volatile gint pending_idle_toggles;
+static std::set<ObjectInstance *> dissociate_list;
 
 GJS_DEFINE_PRIV_FROM_JS(ObjectInstance, gjs_object_instance_class)
 
@@ -900,8 +902,8 @@ wrapped_gobj_dispose_notify(gpointer      data,
 #endif
 
 static void
-gobj_no_longer_kept_alive_func(JSObject *obj,
-                               void     *data)
+gobj_no_longer_kept_alive_func(JS::HandleObject obj,
+                               void            *data)
 {
     ObjectInstance *priv;
 
@@ -909,9 +911,11 @@ gobj_no_longer_kept_alive_func(JSObject *obj,
 
     gjs_debug_lifecycle(GJS_DEBUG_GOBJECT,
                         "GObject wrapper %p will no longer be kept alive, eligible for collection",
-                        obj);
+                        obj.get());
 
-    priv->keep_alive = NULL;
+    priv->keep_alive.reset();
+    size_t erased = dissociate_list.erase(priv);
+    g_assert(erased > 0);
 }
 
 static GQuark
@@ -984,18 +988,16 @@ handle_toggle_down(GObject *gobj)
 
     gjs_debug_lifecycle(GJS_DEBUG_GOBJECT,
                         "Toggle notify gobj %p obj %p is_last_ref true keep-alive %p",
-                        gobj, obj, priv->keep_alive);
+                        gobj, obj, priv->keep_alive.get());
 
     /* Change to weak ref so the wrapper-wrappee pair can be
      * collected by the GC
      */
-    if (priv->keep_alive != NULL) {
+    if (priv->keep_alive.rooted()) {
         gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "Removing object from keep alive");
-        gjs_keep_alive_remove_child(priv->keep_alive,
-                                    gobj_no_longer_kept_alive_func,
-                                    obj,
-                                    priv);
-        priv->keep_alive = NULL;
+        priv->keep_alive.reset();
+        size_t erased = dissociate_list.erase(priv);
+        g_assert(erased > 0);
     }
 }
 
@@ -1018,21 +1020,22 @@ handle_toggle_up(GObject   *gobj)
 
     gjs_debug_lifecycle(GJS_DEBUG_GOBJECT,
                         "Toggle notify gobj %p obj %p is_last_ref false keep-alive %p",
-                        gobj, obj, priv->keep_alive);
+                        gobj, obj, priv->keep_alive.get());
 
     /* Change to strong ref so the wrappee keeps the wrapper alive
      * in case the wrapper has data in it that the app cares about
      */
-    if (priv->keep_alive == NULL) {
+    if (!priv->keep_alive.rooted()) {
+        priv->keep_alive.reset();
         /* FIXME: thread the context through somehow. Maybe by looking up
          * the compartment that obj belongs to. */
         GjsContext *context = gjs_context_get_current();
         gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "Adding object to keep alive");
-        priv->keep_alive = gjs_keep_alive_get_global((JSContext*) gjs_context_get_native_context(context));
-        gjs_keep_alive_add_child(priv->keep_alive,
-                                 gobj_no_longer_kept_alive_func,
-                                 obj,
-                                 priv);
+        auto cx = static_cast<JSContext *>(gjs_context_get_native_context(context));
+        priv->keep_alive.root(cx, obj, gobj_no_longer_kept_alive_func, priv);
+        bool inserted;
+        std::tie(std::ignore, inserted) = dissociate_list.insert(priv);
+        g_assert(inserted);
     }
 }
 
@@ -1240,35 +1243,27 @@ release_native_object (ObjectInstance *priv)
  * pending toggle references.
  */
 void
-gjs_object_prepare_shutdown (JSContext *context)
+gjs_object_clear_toggles(void)
 {
-    JSObject *keep_alive = gjs_keep_alive_get_global_if_exists (context);
-    GjsKeepAliveIter kiter;
-    JSObject *child;
-    void *data;
-
-    if (!keep_alive)
-        return;
-
-    /* First, get rid of anything left over on the main context */
     while (g_main_context_pending(NULL) &&
            g_atomic_int_get(&pending_idle_toggles) > 0) {
         g_main_context_iteration(NULL, false);
     }
+}
+
+void
+gjs_object_prepare_shutdown (JSContext *context)
+{
+    /* First, get rid of anything left over on the main context */
+    gjs_object_clear_toggles();
 
     /* Now, we iterate over all of the objects, breaking the JS <-> C
-     * associaton.  We avoid the potential recursion implied in:
+     * association.  We avoid the potential recursion implied in:
      *   toggle ref removal -> gobj dispose -> toggle ref notify
      * by simply ignoring toggle ref notifications during this process.
      */
-    gjs_keep_alive_iterator_init(&kiter, keep_alive);
-    while (gjs_keep_alive_iterator_next(&kiter,
-                                        gobj_no_longer_kept_alive_func,
-                                        &child, &data)) {
-        ObjectInstance *priv = (ObjectInstance*)data;
-
-        release_native_object(priv);
-    }
+    for (auto iter : dissociate_list)
+        release_native_object(iter);
 }
 
 static ObjectInstance *
@@ -1281,6 +1276,7 @@ init_object_private (JSContext       *context,
     JS_BeginRequest(context);
 
     priv = g_slice_new0(ObjectInstance);
+    new (priv) ObjectInstance();
 
     GJS_INC_COUNTER(object);
 
@@ -1331,12 +1327,10 @@ associate_js_gobject (JSContext       *context,
      * the wrapper to be garbage collected (and thus unref the
      * wrappee).
      */
-    priv->keep_alive = gjs_keep_alive_get_global(context);
-    gjs_keep_alive_add_child(priv->keep_alive,
-                             gobj_no_longer_kept_alive_func,
-                             object,
-                             priv);
-
+    priv->keep_alive.root(context, object, gobj_no_longer_kept_alive_func, priv);
+    bool inserted;
+    std::tie(std::ignore, inserted) = dissociate_list.insert(priv);
+    g_assert(inserted);
     g_object_add_toggle_ref(gobj, wrapped_gobj_toggle_notify, NULL);
 }
 
@@ -1558,7 +1552,7 @@ object_instance_finalize(JSFreeOp  *fop,
         release_native_object(priv);
     }
 
-    if (priv->keep_alive != NULL) {
+    if (priv->keep_alive.rooted()) {
         /* This happens when the refcount on the object is still >1,
          * for example with global objects GDK never frees like GdkDisplay,
          * when we close down the JS runtime.
@@ -1569,10 +1563,9 @@ object_instance_finalize(JSFreeOp  *fop,
         gjs_debug_lifecycle(GJS_DEBUG_GOBJECT,
                             "Removing from keep alive");
 
-        gjs_keep_alive_remove_child(priv->keep_alive,
-                                    gobj_no_longer_kept_alive_func,
-                                    obj,
-                                    priv);
+        priv->keep_alive.reset();
+        size_t erased = dissociate_list.erase(priv);
+        g_assert(erased > 0);
     }
 
     if (priv->info) {
@@ -1586,6 +1579,7 @@ object_instance_finalize(JSFreeOp  *fop,
     }
 
     GJS_DEC_COUNTER(object);
+    priv->~ObjectInstance();
     g_slice_free(ObjectInstance, priv);
 }
 
@@ -2094,6 +2088,7 @@ gjs_define_object_class(JSContext              *context,
 
     GJS_INC_COUNTER(object);
     priv = g_slice_new0(ObjectInstance);
+    new (priv) ObjectInstance();
     priv->info = info;
     if (info)
         g_base_info_ref((GIBaseInfo*) info);
diff --git a/gi/object.h b/gi/object.h
index 92744e1..d37d1df 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -58,6 +58,8 @@ bool      gjs_typecheck_is_object(JSContext       *context,
 
 void      gjs_object_prepare_shutdown   (JSContext     *context);
 
+void gjs_object_clear_toggles(void);
+
 void gjs_object_define_static_methods(JSContext       *context,
                                       JS::HandleObject constructor,
                                       GType            gtype,
diff --git a/gjs/context.cpp b/gjs/context.cpp
index a75c938..1610b26 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -400,6 +400,8 @@ gjs_context_dispose(GObject *object)
         gjs_debug(GJS_DEBUG_CONTEXT,
                   "Destroying JS context");
 
+        gjs_object_clear_toggles();
+
         JS_BeginRequest(js_context->context);
 
         /* Do a full GC here before tearing down, since once we do
diff --git a/gjs/jsapi-util-root.h b/gjs/jsapi-util-root.h
index 444d05b..703b52a 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -263,6 +263,8 @@ public:
         g_assert(!m_rooted);
         GjsHeapOperation<T>::trace(tracer, &m_heap, name);
     }
+
+    bool rooted(void) { return m_rooted; }
 };
 
 #endif /* GJS_JSAPI_UTIL_ROOT_H */