[gjs/wip/ptomato/mozjs38: 11/32] WIP - remove keep-alive



commit 701f9e7fd1dfa08b6ffd5edd6068894883a5fd63
Author: Philip Chimento <philip endlessm com>
Date:   Thu Feb 2 16:19:21 2017 -0800

    WIP - remove keep-alive
    
    FIXME: Maybe this should be squashed into the GjsMaybeOwned patch.
    FIXME: Maybe needs some tests?
    FIXME: Still needs a commit message.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776966

 gi/keep-alive.cpp     |  444 -------------------------------------------------
 gi/keep-alive.h       |   92 ----------
 gi/object.cpp         |   80 ++++-----
 gjs-srcs.mk           |    2 -
 gjs/context.cpp       |    2 +
 gjs/jsapi-util-root.h |    2 +
 6 files changed, 37 insertions(+), 585 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 776ca4d..8a01511 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"
@@ -39,9 +40,9 @@
 #include "proxyutils.h"
 #include "param.h"
 #include "value.h"
-#include "keep-alive.h"
 #include "closure.h"
 #include "gjs_gi_trace.h"
+#include "gjs/jsapi-util-root.h"
 #include "gjs/jsapi-wrapper.h"
 #include "gjs/context-private.h"
 #include "gjs/mem.h"
@@ -54,7 +55,7 @@
 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 */
@@ -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);
     }
 }
 
@@ -1251,14 +1254,6 @@ gjs_object_clear_toggles(void)
 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 */
     gjs_object_clear_toggles();
 
@@ -1267,14 +1262,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 (auto iter : dissociate_list)
+        release_native_object(iter);
 }
 
 static ObjectInstance *
@@ -1338,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);
 }
 
@@ -1565,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.
@@ -1576,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) {
diff --git a/gjs-srcs.mk b/gjs-srcs.mk
index 7c6dc81..2295bf8 100644
--- a/gjs-srcs.mk
+++ b/gjs-srcs.mk
@@ -32,8 +32,6 @@ gjs_srcs =                            \
        gi/gtype.h                      \
        gi/interface.cpp                \
        gi/interface.h                  \
-       gi/keep-alive.cpp               \
-       gi/keep-alive.h                 \
        gi/ns.cpp                       \
        gi/ns.h                         \
        gi/object.cpp                   \
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 9cc12ea..a8f7c06 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -268,6 +268,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 */


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