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

[Philip Chimento <pchimento src gnome org>]

commit 97b4bb7112e8dadc462852f3bc24ff1d82920fcd
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.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777962

 gi/object.cpp |  111 +++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 72 insertions(+), 39 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 896b273..6b0bebb 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>
@@ -93,6 +94,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<GObject *> weak_pointer_list;
+
 extern struct JSClass gjs_object_instance_class;
 static GThread *gjs_eval_thread;
 static volatile gint pending_idle_toggles;
@@ -107,7 +112,8 @@ static void            set_js_obj   (GObject   *gobj,
                                      JSObject  *obj);
 static void            poison_js_obj(GObject   *gobj);
 
-static void            disassociate_js_gobject (GObject *gobj);
+static void disassociate_js_gobject(ObjectInstance *priv);
+
 static void            invalidate_all_signals (ObjectInstance *priv);
 typedef enum {
     SOME_ERROR_OCCURRED = false,
@@ -886,14 +892,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(where_the_object_was);
+#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,
@@ -1125,10 +1132,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)) {
@@ -1171,12 +1177,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);
@@ -1209,16 +1209,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) {
-                if (JS_IsAboutToBeFinalized(ensure_heap_wrapper(gobj))) {
-                    /* 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);
         }
@@ -1226,9 +1217,22 @@ wrapped_gobj_toggle_notify(gpointer      data,
 }
 
 static void
+null_js_obj(GObject  *gobj)
+{
+    auto heap_wrapper = ensure_heap_wrapper(gobj);
+
+    /* Already dead */
+    if (G_UNLIKELY ((gpointer) heap_wrapper == (gpointer) 1))
+        return;
+
+    *heap_wrapper = NULL;
+    weak_pointer_list.erase(gobj);
+}
+
+static void
 release_native_object (ObjectInstance *priv)
 {
-    set_js_obj(priv->gobj, NULL);
+    null_js_obj(priv->gobj);
     g_object_remove_toggle_ref(priv->gobj, wrapped_gobj_toggle_notify, NULL);
     priv->gobj = NULL;
 }
@@ -1294,6 +1298,38 @@ 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(); ) {
+        auto heap_wrapper = ensure_heap_wrapper(*iter);
+        auto priv = static_cast<ObjectInstance *>(JS_GetPrivate(*heap_wrapper));
+
+        JS_UpdateWeakPointerAfterGC(heap_wrapper);
+
+        if (*heap_wrapper == NULL) {
+            /* 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);
+        } else {
+            iter++;
+        }
+    }
+}
+
+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)
@@ -1303,12 +1339,14 @@ associate_js_gobject (JSContext       *context,
     priv = priv_from_js(context, object);
     priv->gobj = gobj;
 
+    ensure_weak_pointer_callback(context);
+
     g_assert(peek_js_obj(gobj) == NULL);
     set_js_obj(gobj, object);
 
-#if DEBUG_DISPOSE
-    g_object_weak_ref(gobj, wrapped_gobj_dispose_notify, object);
-#endif
+    weak_pointer_list.insert(gobj);
+
+    g_object_weak_ref(gobj, wrapped_gobj_dispose_notify, NULL);
 
     /* OK, here is where things get complicated. We want the
      * wrapped gobj to keep the JSObject* wrapper alive, because
@@ -1329,13 +1367,15 @@ associate_js_gobject (JSContext       *context,
 }
 
 static void
-disassociate_js_gobject (GObject   *gobj)
+disassociate_js_gobject (ObjectInstance *priv)
 {
-    JSObject *object;
-    ObjectInstance *priv;
+    GObject *gobj = priv->gobj;
+
+    /* Mark that a JS object once existed, but it doesn't any more */
+    poison_js_obj(gobj);
+
+    g_object_weak_unref(gobj, wrapped_gobj_dispose_notify, NULL);
 
-    object = peek_js_obj(gobj);
-    priv = (ObjectInstance*) JS_GetPrivate(object);
     /* Idles are already checked in the only caller of this
        function, the toggle ref notify, but let's check again...
     */
@@ -1344,13 +1384,6 @@ disassociate_js_gobject (GObject   *gobj)
 
     invalidate_all_signals(priv);
     release_native_object(priv);
-
-    /* Mark that a JS object once existed, but it doesn't any more */
-    poison_js_obj(gobj);
-
-#if DEBUG_DISPOSE
-    g_object_weak_unref(gobj, wrapped_gobj_dispose_notify, object);
-#endif
 }
 
 static bool
@@ -2141,7 +2174,7 @@ static void
 set_js_obj(GObject  *gobj,
            JSObject *obj)
 {
-    ensure_heap_wrapper(gobj)->set(obj);
+    *ensure_heap_wrapper(gobj) = obj;
 }
 
 static void