[gjs/wip/ptomato/mozjs38: 1/3] WIP - object: Update weak pointers after GC

[Philip Chimento <pchimento src gnome org>]

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

    WIP - object: Update weak pointers after GC
    
    The only place where we currently maintain a weak pointer is object.cpp,
    where a GObject is associated with its JS 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.
    
    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
    that processes all the weak pointers it knows about.
    
    FIXME: The weak pointer list might have to have a lock.
    FIXME: Still tests failing.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777962

 gi/object.cpp         |   78 ++++++++++++++++++++++++++++++-------------------
 gjs/jsapi-util-root.h |    3 +-
 2 files changed, 50 insertions(+), 31 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 14fed6d..1fb305b 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -23,6 +23,7 @@
 
 #include <config.h>
 
+#include <set>
 #include <memory>
 #include <stack>
 #include <string.h>
@@ -95,6 +96,10 @@ enum {
 static std::stack<JS::PersistentRootedObject> object_init_list;
 static GHashTable *class_init_properties;
 
+static bool weak_pointer_callback = false;
+/* FIXME: does this need to have a lock? */
+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;
@@ -905,14 +910,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,
@@ -1133,10 +1139,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)) {
@@ -1179,12 +1184,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);
@@ -1217,17 +1216,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);
         }
@@ -1303,6 +1292,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)
@@ -1316,9 +1334,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
@@ -1339,10 +1358,13 @@ 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);
+    weak_pointer_list.erase(priv);
+
     /* Idles are already checked in the only caller of this
        function, the toggle ref notify, but let's check again...
     */
@@ -1354,10 +1376,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 dc73a45..8cb3705 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);
     }
 };