Bug 1285034 - Do not create a null reference when tracing; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Thu, 07 Jul 2016 08:59:34 -0700
changeset 304135 b2354d420c2ca6b8ecef1c8cc7a1ed17cca6e1bf
parent 304134 0a5427e43061dfa61ab18f9b5654227f8ef8c0b4
child 304136 7975518290c621749e1e2111f2dd39f093681dea
push id30414
push usercbook@mozilla.com
push dateFri, 08 Jul 2016 09:59:01 +0000
treeherdermozilla-central@45682df2d2d4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1285034
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1285034 - Do not create a null reference when tracing; r=jonco
js/src/jsapi.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1831,25 +1831,29 @@ JS_NewGlobalObject(JSContext* cx, const 
     return GlobalObject::new_(cx, Valueify(clasp), principals, hookOption, options);
 }
 
 JS_PUBLIC_API(void)
 JS_GlobalObjectTraceHook(JSTracer* trc, JSObject* global)
 {
     MOZ_ASSERT(global->is<GlobalObject>());
 
-    // Off thread parsing and compilation tasks create a dummy global which is then
-    // merged back into the host compartment. Since it used to be a global, it will still
-    // have this trace hook, but it does not have a meaning relative to its new compartment.
-    // We can safely skip it.
-    if (!global->isOwnGlobal())
+    // Off thread parsing and compilation tasks create a dummy global which is
+    // then merged back into the host compartment. Since it used to be a
+    // global, it will still have this trace hook, but it does not have a
+    // meaning relative to its new compartment. We can safely skip it.
+    //
+    // Similarly, if we GC when creating the global, we may not have set that
+    // global's compartment's global pointer yet. In this case, the compartment
+    // will not yet contain anything that needs to be traced.
+    if (!global->isOwnGlobal(trc))
         return;
 
-    // Trace the compartment for any GC things that should only stick around if we know the
-    // compartment is live.
+    // Trace the compartment for any GC things that should only stick around if
+    // we know the compartment is live.
     global->compartment()->trace(trc);
 
     if (JSTraceOp trace = global->compartment()->creationOptions().getTrace())
         trace(trc, global);
 }
 
 JS_PUBLIC_API(void)
 JS_FireOnNewGlobalObject(JSContext* cx, JS::HandleObject global)
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -461,17 +461,30 @@ class JSObject : public js::gc::Cell
     /*
      * Get the enclosing scope of an object. When called on non-scope object,
      * this will just be the global (the name "enclosing scope" still applies
      * in this situation because non-scope objects can be on the scope chain).
      */
     inline JSObject* enclosingScope() const;
 
     inline js::GlobalObject& global() const;
-    inline bool isOwnGlobal() const;
+
+    // In some rare cases the global object's compartment's global may not be
+    // the same global object. For this reason, we need to take extra care when
+    // tracing.
+    //
+    // These cases are:
+    //  1) The off-thread parsing task uses a dummy global since it cannot
+    //     share with the actual global being used concurrently on the main
+    //     thread.
+    //  2) A GC may occur when creating the GlobalObject, in which case the
+    //     compartment global pointer may not yet be set. In this case there is
+    //     nothing interesting to trace in the compartment.
+    inline bool isOwnGlobal(JSTracer*) const;
+    inline js::GlobalObject* globalForTracing(JSTracer*) const;
 
     /*
      * ES5 meta-object properties and operations.
      */
 
   public:
     // Indicates whether a non-proxy is extensible.  Don't call on proxies!
     // This method really shouldn't exist -- but there are a few internal
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -416,20 +416,26 @@ JSObject::global() const
      * The global is read-barriered so that it is kept live by access through
      * the JSCompartment. When accessed through a JSObject, however, the global
      * will be already be kept live by the black JSObject's parent pointer, so
      * does not need to be read-barriered.
      */
     return *compartment()->unsafeUnbarrieredMaybeGlobal();
 }
 
+inline js::GlobalObject*
+JSObject::globalForTracing(JSTracer*) const
+{
+    return compartment()->unsafeUnbarrieredMaybeGlobal();
+}
+
 inline bool
-JSObject::isOwnGlobal() const
+JSObject::isOwnGlobal(JSTracer* trc) const
 {
-    return &global() == this;
+    return globalForTracing(trc) == this;
 }
 
 inline bool
 JSObject::hasAllFlags(js::BaseShape::Flag flags) const
 {
     MOZ_ASSERT(flags);
     if (js::Shape* shape = maybeShape())
         return shape->hasAllObjectFlags(flags);