[gjs/wip/ptomato/mozjs52: 18/20] js: Unbarriered read while in weak ptr callback



commit 1266a3e8b4568fcf233adc2243f8cb158fd14391
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Jun 4 16:13:53 2017 -0400

    js: Unbarriered read while in weak ptr callback
    
    Inside the weak pointer callback, we only need to compare the pointer to
    nullptr. Since we don't actually use the pointer's location, no read
    barrier is needed.
    
    Previously this was not a problem, but SpiderMonkey 52 now asserts that
    the heap is not active when exposing a pointer to active JS through a
    read barrier, so the weak pointer callback will crash if we try to use a
    read barrier.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784196

 gi/closure.cpp        |    8 ++++----
 gi/gtype.cpp          |    5 ++++-
 gjs/jsapi-util-root.h |   12 +++++++++++-
 3 files changed, 19 insertions(+), 6 deletions(-)
---
diff --git a/gi/closure.cpp b/gi/closure.cpp
index 54fe80e..4173f98 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -87,7 +87,7 @@ invalidate_js_pointers(GjsClosure *gc)
 
     c = &gc->priv;
 
-    if (c->obj == NULL)
+    if (c->obj == nullptr)
         return;
 
     c->obj.reset();
@@ -159,7 +159,7 @@ closure_invalidated(gpointer data,
     gjs_debug_closure("Invalidating closure %p which calls object %p",
                       closure, c->obj.get());
 
-    if (c->obj == NULL) {
+    if (c->obj == nullptr) {
         gjs_debug_closure("   (closure %p already dead, nothing to do)",
                           closure);
         return;
@@ -216,7 +216,7 @@ gjs_closure_invoke(GClosure                   *closure,
 
     c = &((GjsClosure*) closure)->priv;
 
-    if (c->obj == NULL) {
+    if (c->obj == nullptr) {
         /* We were destroyed; become a no-op */
         c->context = NULL;
         return;
@@ -296,7 +296,7 @@ gjs_closure_trace(GClosure *closure,
 
     c = &((GjsClosure*) closure)->priv;
 
-    if (c->obj == NULL)
+    if (c->obj == nullptr)
         return;
 
     c->obj.trace(tracer, "signal connection");
diff --git a/gi/gtype.cpp b/gi/gtype.cpp
index ac024b8..411f092 100644
--- a/gi/gtype.cpp
+++ b/gi/gtype.cpp
@@ -65,7 +65,10 @@ update_gtype_weak_pointers(JSContext     *cx,
     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)
+
+        /* No read barriers are needed if the only thing we are doing with the
+         * pointer is comparing it to nullptr. */
+        if (heap_wrapper->unbarrieredGet() == nullptr)
             iter = weak_pointer_list.erase(iter);
         else
             iter++;
diff --git a/gjs/jsapi-util-root.h b/gjs/jsapi-util-root.h
index 6c6462f..33f748d 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -75,7 +75,7 @@ struct GjsHeapOperation<JSObject *> {
     update_after_gc(JS::Heap<JSObject *> *location)
     {
         JS_UpdateWeakPointerAfterGC(location);
-        return (*location == nullptr);
+        return (location->unbarrieredGet() == nullptr);
     }
 };
 
@@ -197,6 +197,16 @@ public:
     }
     inline bool operator!=(const T& other) const { return !(*this == other); }
 
+    /* We can access the pointer without a read barrier if the only thing we
+     * are doing with it is comparing it to nullptr. */
+    bool
+    operator==(std::nullptr_t) const
+    {
+        if (m_rooted)
+            return m_root->get() == nullptr;
+        return m_heap.unbarrieredGet() == nullptr;
+    }
+
     /* You can get a Handle<T> if the thing is rooted, so that you can use this
      * wrapper with stack rooting. However, you must not do this if the
      * JSContext can be destroyed while the Handle is live. */


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