[gjs/wip/ptomato/mozjs38: 4/5] WIP - refactor keep-alive



commit 955ba71b74716655d5669570e32742f9bf3a0a19
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Jan 8 22:25:59 2017 -0800

    WIP - refactor keep-alive
    
    Keep-alive would need to change in SpiderMonkey 38 (and really should have
    already for SpiderMonkey 31, but I didn't realize!) because it uses
    JSObject pointers to generate its hash values. Since the GC can move
    JSObjects around, that is not a good thing to use for a hash value.
    
    This makes keep-alive a lot simpler by using JS::PersistentRootedObject
    (which does not get moved around by the GC). Instead of being a JSObject
    itself, GjsKeepAlive is a pure C++ object, a set which stores
    GjsKeepAliveObjects.
    
    The GjsKeepAliveObject is just JS::PersistentRootedObject augmented with
    a destroy notify function and a hash value.
    
    Instead of sticking all the objects in a big keep-alive object that's
    kept on the global object, this allows each translation unit to have its
    own GjsKeepAlive. This allows us to get rid of the awkward iteration
    functions where you have to skip over objects added by other code by
    keying off their destroy notify functions.
    
    GjsKeepAlive is a std::unordered_set, so uses the normal standard library
    API. It offers two additional methods, add_object() and remove_object(),
    so you don't have to deal with adding an extra root to an object just to
    add or remove it from the keep-alive.
    
    GjsKeepAlive registers itself with the GjsContext and unroots any
    remaining objects when the GjsContext is destroyed.
    
    TODO: Write tests after validating this approach.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776966

 gi/closure.cpp        |   14 +-
 gi/keep-alive.cpp     |  429 +++++++------------------------------------------
 gi/keep-alive.h       |   95 ++++++-----
 gi/object.cpp         |   51 ++----
 gjs/context-private.h |    4 +
 gjs/context.cpp       |   22 +++
 util/glib.cpp         |   45 -----
 util/glib.h           |    3 -
 8 files changed, 162 insertions(+), 501 deletions(-)
---
diff --git a/gi/closure.cpp b/gi/closure.cpp
index cdae2d2..829a436 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -37,9 +37,12 @@ typedef struct {
     JSRuntime *runtime;
     JSContext *context;
     JS::Heap<JSObject *> obj;
+    const GjsKeepAliveObject *keep_alive;  /* NULL if not in keep_alive_list */
     guint unref_on_global_object_finalized : 1;
 } Closure;
 
+static GjsKeepAlive keep_alive_list;
+
 /*
  * Memory management of closures is "interesting" because we're keeping around
  * a JSContext* and then trying to use it spontaneously from the main loop.
@@ -221,10 +224,7 @@ closure_invalidated(gpointer data,
         gjs_debug_closure("   (closure %p's context was alive, "
                           "removing our destroy notifier on global object)",
                           closure);
-        gjs_keep_alive_remove_global_child(c->context,
-                                           global_context_finalized,
-                                           c->obj,
-                                           c);
+        keep_alive_list.remove_object(c->keep_alive);
 
         c->obj = NULL;
         c->context = NULL;
@@ -366,10 +366,8 @@ gjs_closure_new(JSContext  *context,
 
     if (root_function) {
         /* Fully manage closure lifetime if so asked */
-        gjs_keep_alive_add_global_child(context,
-                                        global_context_finalized,
-                                        c->obj,
-                                        c);
+        c->keep_alive = keep_alive_list.add_object(context, c->obj,
+                                                   global_context_finalized, c);
 
         g_closure_add_invalidate_notifier(&c->base, NULL, closure_invalidated);
     } else {
diff --git a/gi/keep-alive.cpp b/gi/keep-alive.cpp
index 4060935..aad3ddf 100644
--- a/gi/keep-alive.cpp
+++ b/gi/keep-alive.cpp
@@ -1,6 +1,7 @@
 /* -*- mode: C; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
 /*
  * Copyright (c) 2008  litl, LLC
+ * Copyright (c) 2016  Philip Chimento
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to
@@ -21,405 +22,97 @@
  * IN THE SOFTWARE.
  */
 
-#include <config.h>
+#include <glib.h>
 
+#include "gjs/context.h"
+#include "gjs/context-private.h"
 #include "gjs/jsapi-wrapper.h"
 #include "keep-alive.h"
-
 #include <util/log.h>
-#include <util/glib.h>
-
-typedef struct {
-    GjsUnrootedFunc notify;
-    JSObject *child;
-    void *data;
-} Child;
-
-typedef struct {
-    GHashTable *children;
-    unsigned int inside_finalize : 1;
-    unsigned int inside_trace : 1;
-} KeepAlive;
-
-extern struct JSClass gjs_keep_alive_class;
 
-GJS_DEFINE_PRIV_FROM_JS(KeepAlive, gjs_keep_alive_class)
-
-static guint
-child_hash(gconstpointer  v)
+/* We use the location of the PersistentRootedObject to generate the hash value,
+ * since, unlike the JSObject pointer, it will not be moved by the GC. */
+size_t GjsKeepAliveObjectHash::
+operator()(const GjsKeepAliveObject& self) const
 {
-    const Child *child = (const Child *) v;
-
     return
-        GPOINTER_TO_UINT(child->notify) ^
-        GPOINTER_TO_UINT(child->child) ^
-        GPOINTER_TO_UINT(child->data);
+        std::hash<GjsUnrootedFunc>{}(self.notify) ^
+        std::hash<const GjsKeepAliveObject *>{}(&self) ^
+        std::hash<void *>{}(self.data);
 }
 
-static gboolean
-child_equal (gconstpointer  v1,
-             gconstpointer  v2)
+bool GjsKeepAliveObject::
+operator==(const GjsKeepAliveObject& other) const
 {
-    const Child *child1 = (const Child *) v1;
-    const Child *child2 = (const Child *) v2;
-
     /* notify is most likely to be equal, so check it last */
-    return child1->data == child2->data &&
-        child1->child == child2->child &&
-        child1->notify == child2->notify;
+    return data == other.data &&
+        get() == other.get() &&
+        notify == other.notify;
 }
 
-static void
-child_free(void *data)
+GjsKeepAlive::
+~GjsKeepAlive()
 {
-    Child *child = (Child *) data;
-    g_slice_free(Child, child);
-}
-
-GJS_NATIVE_CONSTRUCTOR_DEFINE_ABSTRACT(keep_alive)
-
-static void
-keep_alive_finalize(JSFreeOp *fop,
-                    JSObject *obj)
-{
-    KeepAlive *priv;
-    void *key;
-    void *value;
-
-    priv = (KeepAlive *) JS_GetPrivate(obj);
-
     gjs_debug_lifecycle(GJS_DEBUG_KEEP_ALIVE,
-                        "keep_alive finalizing, obj %p priv %p", obj, priv);
-
-    if (priv == NULL)
-        return; /* we are the prototype, not a real instance */
-
-    priv->inside_finalize = true;
-
-    while (gjs_g_hash_table_steal_one(priv->children,
-                                      &key, &value)) {
-        Child *child = (Child *) value;
-        if (child->notify)
-            (* child->notify) (child->child, child->data);
-
-        child_free(child);
-    }
-
-    g_hash_table_destroy(priv->children);
-    g_slice_free(KeepAlive, priv);
+                        "keep_alive finalizing, obj %p", this);
 }
 
-static void
-trace_foreach(void *key,
-              void *value,
-              void *data)
+GjsKeepAlive::
+GjsKeepAlive() : registered_with(nullptr)
 {
-    Child *child = (Child *) value;
-    JSTracer *tracer = (JSTracer *) data;
-
-    if (child->child != NULL) {
-        JS::Value val;
-        val = JS::ObjectValue(*(child->child));
-        JS_CallValueTracer(tracer, &val, "keep-alive::val");
-    }
-}
-
-static void
-keep_alive_trace(JSTracer *tracer,
-                 JSObject *obj)
-{
-    KeepAlive *priv;
-
-    priv = (KeepAlive *) JS_GetPrivate(obj);
-
-    if (priv == NULL) /* prototype */
-        return;
-
-    g_assert(!priv->inside_trace);
-    priv->inside_trace = true;
-    g_hash_table_foreach(priv->children, trace_foreach, tracer);
-    priv->inside_trace = false;
-}
-
-/* The bizarre thing about this vtable is that it applies to both
- * instances of the object, and to the prototype that instances of the
- * class have.
- */
-struct JSClass gjs_keep_alive_class = {
-    "__private_GjsKeepAlive", /* means "new __private_GjsKeepAlive()" works */
-    JSCLASS_HAS_PRIVATE,
-    JS_PropertyStub,
-    JS_DeletePropertyStub,
-    JS_PropertyStub,
-    JS_StrictPropertyStub,
-    JS_EnumerateStub,
-    JS_ResolveStub,
-    JS_ConvertStub,
-    keep_alive_finalize,
-    NULL,
-    NULL,
-    NULL,
-    keep_alive_trace,
-};
-
-JSPropertySpec gjs_keep_alive_proto_props[] = {
-    JS_PS_END
-};
-
-JSFunctionSpec gjs_keep_alive_proto_funcs[] = {
-    JS_FS_END
-};
-
-static JSObject *
-gjs_keep_alive_new(JSContext *context)
-{
-    KeepAlive *priv;
-    bool found;
-
-    /* This function creates an unattached KeepAlive object; following our
-     * general strategy, we have a single KeepAlive class with a constructor
-     * stored on our single "load global" pseudo-global object, and we create
-     * instances with the load global as parent.
+    /* This function creates an unregistered GjsKeepAlive object. It will be
+     * registered with a GjsContext the first time any object is added to it.
+     * Once it is registered, then at the time the context is destroyed, any
+     * remaining objects will be unrooted so that we don't fail our memory leak
+     * checks in the tests.
      */
-
-    g_assert(context != NULL);
-
-    JSAutoRequest ar(context);
-
-    JS::RootedObject global(context, gjs_get_import_global(context));
-
-    g_assert(global != NULL);
-
-    if (!JS_HasProperty(context, global, gjs_keep_alive_class.name, &found))
-        return NULL;
-
-    if (!found) {
-        JSObject *prototype;
-
-        gjs_debug(GJS_DEBUG_KEEP_ALIVE,
-                  "Initializing keep-alive class in context %p global %p",
-                  context, global.get());
-
-        prototype = JS_InitClass(context, global,
-                                 /* parent prototype JSObject* for
-                                  * prototype; NULL for
-                                  * Object.prototype
-                                  */
-                                 JS::NullPtr(),
-                                 &gjs_keep_alive_class,
-                                 /* constructor for instances (NULL for
-                                  * none - just name the prototype like
-                                  * Math - rarely correct)
-                                  */
-                                 gjs_keep_alive_constructor,
-                                 /* number of constructor args */
-                                 0,
-                                 /* props of prototype */
-                                 &gjs_keep_alive_proto_props[0],
-                                 /* funcs of prototype */
-                                 &gjs_keep_alive_proto_funcs[0],
-                                 /* props of constructor, MyConstructor.myprop */
-                                 NULL,
-                                 /* funcs of constructor, MyConstructor.myfunc() */
-                                 NULL);
-        if (prototype == NULL)
-            g_error("Can't init class %s", gjs_keep_alive_class.name);
-
-        gjs_debug(GJS_DEBUG_KEEP_ALIVE, "Initialized class %s prototype %p",
-                  gjs_keep_alive_class.name, prototype);
-    }
-
     gjs_debug(GJS_DEBUG_KEEP_ALIVE,
-              "Creating new keep-alive object for context %p global %p",
-              context, global.get());
-
-    JS::RootedObject keep_alive(context,
-        JS_NewObject(context, &gjs_keep_alive_class, JS::NullPtr(), global));
-    if (keep_alive == NULL) {
-        gjs_log_exception(context);
-        g_error("Failed to create keep_alive object");
-    }
-
-    priv = g_slice_new0(KeepAlive);
-    priv->children = g_hash_table_new_full(child_hash, child_equal, NULL, child_free);
-
-    g_assert(priv_from_js(context, keep_alive) == NULL);
-    JS_SetPrivate(keep_alive, priv);
-
+              "Creating new keep-alive object %p", this);
     gjs_debug_lifecycle(GJS_DEBUG_KEEP_ALIVE,
-                        "keep_alive constructor, obj %p priv %p",
-                        keep_alive.get(), priv);
-
-    return keep_alive;
-}
-
-void
-gjs_keep_alive_add_child(JSObject          *keep_alive,
-                         GjsUnrootedFunc    notify,
-                         JSObject          *obj,
-                         void              *data)
-{
-    KeepAlive *priv;
-    Child *child;
-
-    g_assert(keep_alive != NULL);
-    priv = (KeepAlive *) JS_GetPrivate(keep_alive);
-    g_assert(priv != NULL);
-
-    g_return_if_fail(!priv->inside_trace);
-    g_return_if_fail(!priv->inside_finalize);
-
-    child = g_slice_new0(Child);
-    child->notify = notify;
-    child->child = obj;
-    child->data = data;
-
-    /* this is sort of an expensive check, probably */
-    g_return_if_fail(g_hash_table_lookup(priv->children, child) == NULL);
-
-    /* this overwrites any identical-by-value previous child,
-     * but there should not be one.
-     */
-    g_hash_table_replace(priv->children, child, child);
-}
-
-void
-gjs_keep_alive_remove_child(JSObject          *keep_alive,
-                            GjsUnrootedFunc    notify,
-                            JSObject          *obj,
-                            void              *data)
-{
-    KeepAlive *priv;
-    Child child;
-
-    g_assert(keep_alive != NULL);
-    priv = (KeepAlive *) JS_GetPrivate(keep_alive);
-    g_assert(priv != NULL);
-
-    g_return_if_fail(!priv->inside_trace);
-    g_return_if_fail(!priv->inside_finalize);
-
-    child.notify = notify;
-    child.child = obj;
-    child.data = data;
-
-    g_hash_table_remove(priv->children, &child);
+                        "keep_alive constructor, obj %p", this);
 }
 
-static JSObject*
-gjs_keep_alive_create(JSContext *context)
+void GjsKeepAlive::
+ensure_registered(JSContext *cx)
 {
-    JSObject *keep_alive;
+    if (!registered_with) {
+        auto gjs_context = static_cast<GjsContext *>(JS_GetContextPrivate(cx));
 
-    JS_BeginRequest(context);
+        if (GJS_IS_CONTEXT(gjs_context)) {
+            gjs_debug(GJS_DEBUG_KEEP_ALIVE,
+                      "Registering keep-alive object %p with context %p GjsContext %p",
+                      this, cx, gjs_context);
+            _gjs_context_register_keep_alive(gjs_context, this);
+        } else {
+            g_warning("GjsKeepAlive used with non-GjsContext");
+        }
 
-    keep_alive = gjs_keep_alive_new(context);
-    if (!keep_alive)
-        g_error("could not create keep_alive on global object, no memory?");
-
-    gjs_set_global_slot(context, GJS_GLOBAL_SLOT_KEEP_ALIVE, JS::ObjectValue(*keep_alive));
-
-    JS_EndRequest(context);
-    return keep_alive;
-}
-
-JSObject*
-gjs_keep_alive_get_global_if_exists (JSContext *context)
-{
-    JS::Value keep_alive;
-
-    keep_alive = gjs_get_global_slot(context, GJS_GLOBAL_SLOT_KEEP_ALIVE);
-
-    if (G_LIKELY (keep_alive.isObject()))
-        return &keep_alive.toObject();
-
-    return NULL;
-}
-
-JSObject*
-gjs_keep_alive_get_global(JSContext *context)
-{
-    JSObject *keep_alive = gjs_keep_alive_get_global_if_exists(context);
-
-    if (G_LIKELY(keep_alive))
-        return keep_alive;
-
-    return gjs_keep_alive_create(context);
-}
-
-void
-gjs_keep_alive_add_global_child(JSContext         *context,
-                                GjsUnrootedFunc  notify,
-                                JSObject          *child,
-                                void              *data)
-{
-    JSObject *keep_alive;
-
-    JS_BeginRequest(context);
-
-    keep_alive = gjs_keep_alive_get_global(context);
-
-    gjs_keep_alive_add_child(keep_alive, notify, child, data);
-
-    JS_EndRequest(context);
-}
-
-void
-gjs_keep_alive_remove_global_child(JSContext         *context,
-                                   GjsUnrootedFunc  notify,
-                                   JSObject          *child,
-                                   void              *data)
-{
-    JSObject *keep_alive;
-
-    JS_BeginRequest(context);
-
-    keep_alive = gjs_keep_alive_get_global(context);
-
-    if (!keep_alive)
-        g_error("no keep_alive property on the global object, have you "
-                "previously added this child?");
-
-    gjs_keep_alive_remove_child(keep_alive, notify, child, data);
+        registered_with = cx;
+    }
 
-    JS_EndRequest(context);
+    if (G_UNLIKELY(registered_with != cx))
+        g_critical("You can only put objects from one JSContext in the same "
+                   "keep-alive");
 }
 
-typedef struct {
-    GHashTableIter hashiter;
-} GjsRealKeepAliveIter;
-
-void
-gjs_keep_alive_iterator_init (GjsKeepAliveIter *iter,
-                              JSObject         *keep_alive)
+const GjsKeepAliveObject * GjsKeepAlive::
+add_object(JSContext      *cx,
+           JSObject       *obj,
+           GjsUnrootedFunc notify,
+           void           *data)
 {
-    GjsRealKeepAliveIter *real = (GjsRealKeepAliveIter*)iter;
-    KeepAlive *priv = (KeepAlive *) JS_GetPrivate(keep_alive);
-    g_assert(priv != NULL);
-    g_hash_table_iter_init(&real->hashiter, priv->children);
+    ensure_registered(cx);
+    auto inserted = emplace(cx, obj, notify, data);
+    if (!inserted.second)
+        g_critical("Object inserted twice in keep alive");
+    return &(*inserted.first);
 }
 
-bool
-gjs_keep_alive_iterator_next (GjsKeepAliveIter  *iter,
-                              GjsUnrootedFunc    notify_func,
-                              JSObject         **out_child,
-                              void             **out_data)
+/* Removes an object from the keep-alive list (and zeroes out the pointer passed
+ * in to the function so you don't forget to */
+void GjsKeepAlive::
+remove_object(const GjsKeepAliveObject*& object)
 {
-    GjsRealKeepAliveIter *real = (GjsRealKeepAliveIter*)iter;
-    gpointer k, v;
-    bool ret = false;
-
-    while (g_hash_table_iter_next(&real->hashiter, &k, &v)) {
-        Child *child = (Child*)k;
-
-        if (child->notify != notify_func)
-            continue;
-
-        ret = true;
-        *out_child = child->child;
-        *out_data = child->data;
-        break;
-    }
-
-    return ret;
+    erase(*object);
+    object = NULL;
 }
diff --git a/gi/keep-alive.h b/gi/keep-alive.h
index a9b479b..1dadd6c 100644
--- a/gi/keep-alive.h
+++ b/gi/keep-alive.h
@@ -1,6 +1,7 @@
 /* -*- mode: C; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
 /*
  * Copyright (c) 2008  litl, LLC
+ * Copyright (c) 2016  Philip Chimento
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to
@@ -24,22 +25,23 @@
 #ifndef __GJS_KEEP_ALIVE_H__
 #define __GJS_KEEP_ALIVE_H__
 
-#include <stdbool.h>
+#include <functional>
+#include <unordered_set>
+
 #include <glib.h>
-#include "gjs/jsapi-util.h"
+
+#include "gjs/jsapi-wrapper.h"
 
 G_BEGIN_DECLS
 
-/* This is an alternative to JS::PersistentRooted<T>.
+/* This is a variant of JS::PersistentRootedObject that lets you associate C
+ * types more easily with JSObjects.
  *
  * This "keep alive" object holds a collection of child objects and
- * traces them when GC occurs. If the keep alive object is collected,
- * it calls a notification function on all the child objects.
- *
- * The "global keep alive" is stuck on the global object as a property,
- * so its children only get notified when the entire JSContext is
- * blown away (or its global object replaced, I suppose, but that
- * won't happen afaik).
+ * keeps them rooted with JS::PersistentRootedObject. If a child object is
+ * removed from the keep alive object, it calls a notification function.
+ * When the JSContext is destroyed, any remaining child objects will also be
+ * unrooted and their notification functions called.
  *
  * The problem with JS::PersistentRooted<T> is that it has no notification when
  * the JSContext is destroyed. Also, it can be annoying to wrap a C type purely
@@ -47,46 +49,57 @@ G_BEGIN_DECLS
  *
  * All three fields (notify, child, and data) are optional, so you can have
  * no JSObject - just notification+data - and you can have no notifier,
- * only the keep-alive capability.
+ * only the keep-alive capability (but in that case just use
+ * JS::PersistentRootedObject!)
  */
 
 typedef void (* GjsUnrootedFunc) (JSObject *obj,
                                   void     *data);
 
+G_END_DECLS
+
+struct GjsKeepAliveObject : public JS::PersistentRootedObject {
+    GjsUnrootedFunc notify;
+    void *data;
+
+    GjsKeepAliveObject(JSContext      *cx,
+                       JSObject       *obj,
+                       GjsUnrootedFunc a_notify = nullptr,
+                       void           *a_data   = nullptr)
+    : JS::PersistentRootedObject(cx, obj), notify(a_notify), data(a_data)
+    {}
 
-void      gjs_keep_alive_add_child                 (JSObject          *keep_alive,
-                                                    GjsUnrootedFunc    notify,
-                                                    JSObject          *child,
-                                                    void              *data);
-void      gjs_keep_alive_remove_child              (JSObject          *keep_alive,
-                                                    GjsUnrootedFunc    notify,
-                                                    JSObject          *child,
-                                                    void              *data);
-JSObject* gjs_keep_alive_get_global                (JSContext         *context);
-JSObject* gjs_keep_alive_get_global_if_exists      (JSContext         *context);
-void      gjs_keep_alive_add_global_child          (JSContext         *context,
-                                                    GjsUnrootedFunc  notify,
-                                                    JSObject          *child,
-                                                    void              *data);
-void      gjs_keep_alive_remove_global_child       (JSContext         *context,
-                                                    GjsUnrootedFunc  notify,
-                                                    JSObject          *child,
-                                                    void              *data);
-
-typedef struct GjsKeepAliveIter GjsKeepAliveIter;
-struct GjsKeepAliveIter {
-    gpointer dummy[4];
-    guint v;
-    GHashTableIter dummyhiter;
+    ~GjsKeepAliveObject()
+    {
+        if (notify)
+            notify(get(), data);
+    }
+
+    bool operator==(const GjsKeepAliveObject& other) const;
+    inline bool operator!=(const GjsKeepAliveObject& other) const
+    {
+        return !(*this == other);
+    }
 };
 
-void gjs_keep_alive_iterator_init (GjsKeepAliveIter *iter, JSObject *keep_alive);
+struct GjsKeepAliveObjectHash {
+    size_t operator()(const GjsKeepAliveObject& self) const;
+};
 
-bool gjs_keep_alive_iterator_next (GjsKeepAliveIter  *iter,
-                                   GjsUnrootedFunc    notify_func,
-                                   JSObject         **out_child,
-                                   void             **out_data);
+class GjsKeepAlive : public std::unordered_set<GjsKeepAliveObject, GjsKeepAliveObjectHash> {
+    JSContext *registered_with;
+    void ensure_registered(JSContext *cx);
 
-G_END_DECLS
+public:
+    GjsKeepAlive();
+    ~GjsKeepAlive();
+
+    const GjsKeepAliveObject *add_object(JSContext      *cx,
+                                         JSObject       *obj,
+                                         GjsUnrootedFunc notify = nullptr,
+                                         void           *data   = nullptr);
+
+    void remove_object(const GjsKeepAliveObject*& object);
+};
 
 #endif  /* __GJS_KEEP_ALIVE_H__ */
diff --git a/gi/object.cpp b/gi/object.cpp
index 28e74d6..7855747 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -55,7 +55,7 @@
 typedef struct {
     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 */
+    const GjsKeepAliveObject *keep_alive; /* NULL if we are not added to it */
     GType gtype;
 
     /* a list of all signal connections, used when tracing */
@@ -93,6 +93,8 @@ enum {
 static std::stack<JS::PersistentRootedObject> object_init_list;
 static GHashTable *class_init_properties;
 
+static GjsKeepAlive keep_alive_list;
+
 extern struct JSClass gjs_object_instance_class;
 static GThread *gjs_eval_thread;
 static volatile gint pending_idle_toggles;
@@ -990,11 +992,7 @@ handle_toggle_down(GObject *gobj)
      */
     if (priv->keep_alive != NULL) {
         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;
+        keep_alive_list.remove_object(priv->keep_alive);
     }
 }
 
@@ -1026,12 +1024,12 @@ handle_toggle_up(GObject   *gobj)
         /* FIXME: thread the context through somehow. Maybe by looking up
          * the compartment that obj belongs to. */
         GjsContext *context = gjs_context_get_current();
+        auto cx = static_cast<JSContext *>(gjs_context_get_native_context(context));
         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);
+
+        priv->keep_alive = keep_alive_list.add_object(cx, obj,
+                                                      gobj_no_longer_kept_alive_func,
+                                                      priv);
     }
 }
 
@@ -1241,14 +1239,6 @@ release_native_object (ObjectInstance *priv)
 void
 gjs_object_prepare_shutdown (JSContext *context)
 {
-    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) {
@@ -1260,14 +1250,8 @@ gjs_object_prepare_shutdown (JSContext *context)
      *   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 (const auto& child : keep_alive_list)
+        release_native_object(static_cast<ObjectInstance *>(child.data));
 }
 
 static ObjectInstance *
@@ -1330,11 +1314,9 @@ 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 = keep_alive_list.add_object(context, object,
+                                                  gobj_no_longer_kept_alive_func,
+                                                  priv);
 
     g_object_add_toggle_ref(gobj, wrapped_gobj_toggle_notify, NULL);
 }
@@ -1569,10 +1551,7 @@ 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);
+        keep_alive_list.remove_object(priv->keep_alive);
     }
 
     if (priv->info) {
diff --git a/gjs/context-private.h b/gjs/context-private.h
index 5b24b77..57eb544 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -27,6 +27,7 @@
 #include <inttypes.h>
 
 #include "context.h"
+#include "gi/keep-alive.h"
 
 G_BEGIN_DECLS
 
@@ -37,6 +38,9 @@ void         _gjs_context_schedule_gc_if_needed       (GjsContext *js_context);
 void _gjs_context_exit(GjsContext *js_context,
                        uint8_t     exit_code);
 
+void _gjs_context_register_keep_alive(GjsContext   *js_context,
+                                      GjsKeepAlive *keep_alive);
+
 G_END_DECLS
 
 #endif  /* __GJS_CONTEXT_PRIVATE_H__ */
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 1c3b829..fe74d17 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -23,6 +23,8 @@
 
 #include <config.h>
 
+#include <vector>
+
 #include <gio/gio.h>
 
 #include "context-private.h"
@@ -63,6 +65,7 @@ struct _GjsContext {
     JSRuntime *runtime;
     JSContext *context;
     JS::Heap<JSObject*> global;
+    std::vector<GjsKeepAlive *> keep_alives;
 
     char *program_name;
 
@@ -408,6 +411,10 @@ gjs_context_dispose(GObject *object)
         JS_RemoveExtraGCRootsTracer(js_context->runtime, gjs_context_tracer,
                                     js_context);
 
+        /* Release all the PersistentRooteds */
+        for (auto keep_alive : js_context->keep_alives)
+            keep_alive->clear();
+
         /* Tear down JS */
         JS_DestroyContext(js_context->context);
         js_context->context = NULL;
@@ -883,3 +890,18 @@ gjs_get_import_global(JSContext *context)
     GjsContext *gjs_context = (GjsContext *) JS_GetContextPrivate(context);
     return gjs_context->global;
 }
+
+/*
+ * _gjs_context_register_keep_alive:
+ * @self: the context
+ * @keep_alive: a #GjsKeepAlive object
+ *
+ * Ensures @keep_alive is known to @self so that all the roots can be released
+ * before destroying the context.
+ */
+void
+_gjs_context_register_keep_alive(GjsContext   *self,
+                                 GjsKeepAlive *keep_alive)
+{
+    self->keep_alives.push_back(keep_alive);
+}
diff --git a/util/glib.cpp b/util/glib.cpp
index f084e7f..26caff7 100644
--- a/util/glib.cpp
+++ b/util/glib.cpp
@@ -25,51 +25,6 @@
 
 #include "glib.h"
 
-#include <config.h>
-
-typedef struct {
-    void *key;
-    void *value;
-} StoreOneData;
-
-static gboolean
-get_first_one_predicate(void  *key,
-                        void  *value,
-                        void  *data)
-{
-    StoreOneData *sod = (StoreOneData *) data;
-
-    sod->key = key;
-    sod->value = value;
-
-    /* found it! */
-    return true;
-}
-
-bool
-gjs_g_hash_table_steal_one(GHashTable *hash,
-                           void      **key_p,
-                           void      **value_p)
-{
-    StoreOneData sod;
-
-    sod.key = NULL;
-    sod.value = NULL;
-    g_hash_table_find(hash, get_first_one_predicate, &sod);
-
-    if (sod.key == NULL)
-        return false;
-
-    if (key_p)
-        *key_p = sod.key;
-    if (value_p)
-        *value_p = sod.value;
-
-    g_hash_table_steal(hash, sod.key);
-
-    return sod.value != NULL;
-}
-
 /** gjs_g_strv_concat:
  *
  * Concate an array of string arrays to one string array. The strings in each
diff --git a/util/glib.h b/util/glib.h
index 393cc8a..7522034 100644
--- a/util/glib.h
+++ b/util/glib.h
@@ -28,9 +28,6 @@
 
 G_BEGIN_DECLS
 
-bool     gjs_g_hash_table_steal_one  (GHashTable  *hash,
-                                      void       **key_p,
-                                      void       **value_p);
 char**   gjs_g_strv_concat           (char      ***strv_array,
                                       int          len);
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]