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



commit 9219d6ae08bb4db9dd9f9f453d82d5b176365ede
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.

 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 b314ebb..61bb46b 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -86,7 +86,7 @@ invalidate_js_pointers(GjsClosure *gc)
 
     c = &gc->priv;
 
-    if (c->obj == NULL)
+    if (c->obj == nullptr)
         return;
 
     c->obj.reset();
@@ -157,7 +157,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;
@@ -206,7 +206,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;
@@ -286,7 +286,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 70b7e24..8a13e43 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);
     }
 };
 
@@ -196,6 +196,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]