[gjs/wip/ptomato/mozjs38: 26/28] WIP - js: Update weak pointers after GC

[Philip Chimento <pchimento src gnome org>]

commit 514eb5ee1fca4217f6bce3c9124e1d577c2d595d
Author: Philip Chimento <philip endlessm com>
Date:   Thu Jan 26 16:56:29 2017 -0800

    WIP - js: Update weak pointers after GC
    
    There are two places where we maintain weak pointers to GC things: one is
    in object.cpp, where a GObject is associated with its JS wrapper object.
    The other is in gtype.cpp, where each GType is associated with its
    wrapper object.
    
    In SpiderMonkey 38, we have to call JS_UpdateWeakPointerAfterGC() on weak
    pointers, every garbage collection cycle, in case the garbage collector
    moves them.
    
    FIXME: This is one way to do it, another way might be to add a separate
    callback for each object with JS_AddWeakPointerCallback() instead of one
    callback per file that processes all the weak pointers it knows about.
    
    FIXME: The GType weak pointers might be better off not stored as GType
    qdata, instead have a map<GType, JS::Heap<JSObject*>*>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777962

 gi/gtype.cpp          |   50 +++++++++++++++++++++++++------
 gi/object.cpp         |   77 ++++++++++++++++++++++++++++++-------------------
 gjs/jsapi-util-root.h |    3 +-
 3 files changed, 89 insertions(+), 41 deletions(-)
---
diff --git a/gi/gtype.cpp b/gi/gtype.cpp
index 10ba932..35b9900 100644
--- a/gi/gtype.cpp
+++ b/gi/gtype.cpp
@@ -24,11 +24,16 @@
 
 #include <config.h>
 
+#include <set>
+
 #include "gtype.h"
 #include "gjs/jsapi-wrapper.h"
 #include <util/log.h>
 #include <girepository.h>
 
+static bool weak_pointer_callback = false;
+static std::set<GType> weak_pointer_list;
+
 static JS::Value
 gjs_gtype_create_proto(JSContext       *context,
                        JS::HandleObject module,
@@ -53,6 +58,28 @@ gjs_get_gtype_wrapper_quark(void)
 }
 
 static void
+update_gtype_weak_pointers(JSRuntime *rt,
+                           void      *data)
+{
+    for (auto iter = weak_pointer_list.begin(); iter != weak_pointer_list.end(); ) {
+        auto heap_wrapper = static_cast<JS::Heap<JSObject *> *>(g_type_get_qdata(*iter, 
gjs_get_gtype_wrapper_quark()));
+        JS_UpdateWeakPointerAfterGC(heap_wrapper);
+        if (*heap_wrapper == nullptr)
+            iter = weak_pointer_list.erase(iter);
+        else
+            iter++;
+    }
+}
+
+static void
+ensure_weak_pointer_callback(JSContext *cx)
+{
+    if (!weak_pointer_callback)
+        JS_AddWeakPointerCallback(JS_GetRuntime(cx), update_gtype_weak_pointers,
+                                  nullptr);
+}
+
+static void
 gjs_gtype_finalize(JSFreeOp *fop,
                    JSObject *obj)
 {
@@ -62,6 +89,7 @@ gjs_gtype_finalize(JSFreeOp *fop,
     if (G_UNLIKELY(gtype == 0))
         return;
 
+    weak_pointer_list.erase(gtype);
     g_type_set_qdata(gtype, gjs_get_gtype_wrapper_quark(), NULL);
 }
 
@@ -120,8 +148,6 @@ JSObject *
 gjs_gtype_create_gtype_wrapper (JSContext *context,
                                 GType      gtype)
 {
-    JSObject *object;
-
     JS_BeginRequest(context);
 
     /* put constructor for GIRepositoryGType() in the global namespace */
@@ -129,21 +155,25 @@ gjs_gtype_create_gtype_wrapper (JSContext *context,
     JS::RootedObject proto(context,
         gjs_gtype_create_proto(context, global, "GIRepositoryGType", JS::NullPtr()).toObjectOrNull());
 
-    object = (JSObject*) g_type_get_qdata(gtype, gjs_get_gtype_wrapper_quark());
-    if (object != NULL)
+    auto heap_wrapper =
+        static_cast<JS::Heap<JSObject *> *>(g_type_get_qdata(gtype, gjs_get_gtype_wrapper_quark()));
+    if (heap_wrapper != nullptr)
         goto out;
 
-    object = JS_NewObjectWithGivenProto(context, &gjs_gtype_class, proto,
-                                        JS::NullPtr());
-    if (object == NULL)
+    heap_wrapper = new JS::Heap<JSObject *>();
+    *heap_wrapper = JS_NewObjectWithGivenProto(context, &gjs_gtype_class, proto,
+                                               JS::NullPtr());
+    if (*heap_wrapper == nullptr)
         goto out;
 
-    JS_SetPrivate(object, GSIZE_TO_POINTER(gtype));
-    g_type_set_qdata(gtype, gjs_get_gtype_wrapper_quark(), object);
+    JS_SetPrivate(*heap_wrapper, GSIZE_TO_POINTER(gtype));
+    ensure_weak_pointer_callback(context);
+    g_type_set_qdata(gtype, gjs_get_gtype_wrapper_quark(), heap_wrapper);
+    weak_pointer_list.insert(gtype);
 
  out:
     JS_EndRequest(context);
-    return object;
+    return *heap_wrapper;
 }
 
 static GType
diff --git a/gi/object.cpp b/gi/object.cpp
index ce096b4..a0ba026 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -99,6 +99,9 @@ enum {
 static std::stack<JS::PersistentRootedObject> object_init_list;
 static GHashTable *class_init_properties;
 
+static bool weak_pointer_callback = false;
+static std::set<ObjectInstance *> weak_pointer_list;
+
 extern struct JSClass gjs_object_instance_class;
 static GThread *gjs_eval_thread;
 static volatile gint pending_idle_toggles;
@@ -909,14 +912,15 @@ object_instance_props_to_g_parameters(JSContext                  *context,
 }
 
 #define DEBUG_DISPOSE 0
-#if DEBUG_DISPOSE
 static void
 wrapped_gobj_dispose_notify(gpointer      data,
                             GObject      *where_the_object_was)
 {
-    gjs_debug(GJS_DEBUG_GOBJECT, "JSObject %p GObject %p disposed", data, where_the_object_was);
-}
+    weak_pointer_list.erase(static_cast<ObjectInstance *>(data));
+#if DEBUG_DISPOSE
+    gjs_debug(GJS_DEBUG_GOBJECT, "Wrapped GObject %p disposed", where_the_object_was);
 #endif
+}
 
 static void
 gobj_no_longer_kept_alive_func(JS::HandleObject obj,
@@ -933,6 +937,8 @@ gobj_no_longer_kept_alive_func(JS::HandleObject obj,
     priv->keep_alive.reset();
     size_t erased = dissociate_list.erase(priv);
     g_assert(erased > 0);
+
+    weak_pointer_list.erase(priv);
 }
 
 static GQuark
@@ -1137,10 +1143,9 @@ wrapped_gobj_toggle_notify(gpointer      data,
                            GObject      *gobj,
                            gboolean      is_last_ref)
 {
-    bool is_main_thread, is_sweeping;
+    bool is_main_thread;
     bool toggle_up_queued, toggle_down_queued;
     GjsContext *context;
-    JSContext *js_context;
 
     context = gjs_context_get_current();
     if (_gjs_context_destroying(context)) {
@@ -1183,12 +1188,6 @@ wrapped_gobj_toggle_notify(gpointer      data,
      * not a big deal.
      */
     is_main_thread = (gjs_eval_thread == g_thread_self());
-    if (is_main_thread) {
-        js_context = (JSContext*) gjs_context_get_native_context(context);
-        is_sweeping = gjs_runtime_is_sweeping(JS_GetRuntime(js_context));
-    } else {
-        is_sweeping = false;
-    }
 
     toggle_up_queued = toggle_idle_source_is_queued(gobj, TOGGLE_UP);
     toggle_down_queued = toggle_idle_source_is_queued(gobj, TOGGLE_DOWN);
@@ -1221,17 +1220,7 @@ wrapped_gobj_toggle_notify(gpointer      data,
                 g_error("toggling up object %s that's already queued to toggle up\n",
                         G_OBJECT_TYPE_NAME(gobj));
             }
-            if (is_sweeping) {
-                ObjectInstance *priv = get_object_qdata(gobj);
-                if (priv->keep_alive.update_after_gc()) {
-                    /* Ouch, the JS object is dead already. Disassociate the GObject
-                     * and hope the GObject dies too.
-                     */
-                    disassociate_js_gobject(gobj);
-                }
-            } else {
-                handle_toggle_up(gobj);
-            }
+            handle_toggle_up(gobj);
         } else {
             queue_toggle_idle(gobj, TOGGLE_UP);
         }
@@ -1307,6 +1296,35 @@ init_object_private (JSContext       *context,
 }
 
 static void
+update_heap_wrapper_weak_pointers(JSRuntime *rt,
+                                  gpointer   data)
+{
+    for (auto iter = weak_pointer_list.begin(); iter != weak_pointer_list.end(); ) {
+        ObjectInstance *priv = *iter;
+        if (priv->keep_alive.rooted() || priv->keep_alive == nullptr ||
+            !priv->keep_alive.update_after_gc()) {
+            iter++;
+        } else {
+            /* Ouch, the JS object is dead already. Disassociate the
+             * GObject and hope the GObject dies too. (Remove it from
+             * the weak pointer list first, since the disassociation
+             * may also cause it to be erased.)
+             */
+            iter = weak_pointer_list.erase(iter);
+            disassociate_js_gobject(priv->gobj);
+        }
+    }
+}
+
+static void
+ensure_weak_pointer_callback(JSContext *cx)
+{
+    if (!weak_pointer_callback)
+        JS_AddWeakPointerCallback(JS_GetRuntime(cx),
+                                  update_heap_wrapper_weak_pointers, NULL);
+}
+
+static void
 associate_js_gobject (JSContext       *context,
                       JS::HandleObject object,
                       GObject         *gobj)
@@ -1320,9 +1338,10 @@ associate_js_gobject (JSContext       *context,
 
     set_object_qdata(gobj, priv);
 
-#if DEBUG_DISPOSE
-    g_object_weak_ref(gobj, wrapped_gobj_dispose_notify, object);
-#endif
+    ensure_weak_pointer_callback(context);
+    weak_pointer_list.insert(priv);
+
+    g_object_weak_ref(gobj, wrapped_gobj_dispose_notify, priv);
 
     /* OK, here is where things get complicated. We want the
      * wrapped gobj to keep the JSObject* wrapper alive, because
@@ -1343,10 +1362,12 @@ associate_js_gobject (JSContext       *context,
 }
 
 static void
-disassociate_js_gobject (GObject   *gobj)
+disassociate_js_gobject(GObject *gobj)
 {
     ObjectInstance *priv = get_object_qdata(gobj);
 
+    g_object_weak_unref(priv->gobj, wrapped_gobj_dispose_notify, priv);
+
     /* Idles are already checked in the only caller of this
        function, the toggle ref notify, but let's check again...
     */
@@ -1358,10 +1379,6 @@ disassociate_js_gobject (GObject   *gobj)
 
     /* Mark that a JS object once existed, but it doesn't any more */
     priv->js_object_finalized = true;
-
-#if DEBUG_DISPOSE
-    g_object_weak_unref(gobj, wrapped_gobj_dispose_notify, object);
-#endif
 }
 
 static bool
diff --git a/gjs/jsapi-util-root.h b/gjs/jsapi-util-root.h
index f3cf196..0b1614a 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -86,7 +86,8 @@ struct GjsHeapOperation<JSObject *> {
     static bool
     update_after_gc(JS::Heap<JSObject *> *location)
     {
-        return JS_IsAboutToBeFinalized(location);
+        JS_UpdateWeakPointerAfterGC(location);
+        return (*location == nullptr);
     }
 };