Bug 1458456 part 8 - Fix remaining TSan races. r=jonco
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 04 May 2018 17:29:05 +0200
changeset 473027 1f6d4b0df2226e001496060c4f4ba439e2b1f143
parent 473026 028157451526152028dee71b3546ee6b904e35f0
child 473028 cbdef048e40cdc00eee6079f3761a9c8bd21e60b
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1458456
milestone61.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 1458456 part 8 - Fix remaining TSan races. r=jonco
js/src/gc/Tracer.cpp
js/src/vm/JSCompartment.cpp
js/src/vm/JSCompartment.h
--- a/js/src/gc/Tracer.cpp
+++ b/js/src/gc/Tracer.cpp
@@ -59,34 +59,43 @@ struct DoCallbackFunctor : public Identi
         return js::gc::RewrapTaggedPointer<S, T>::wrap(DoCallback(trc, &t, name));
     }
 };
 
 template <>
 Value
 DoCallback<Value>(JS::CallbackTracer* trc, Value* vp, const char* name)
 {
-    *vp = DispatchTyped(DoCallbackFunctor<Value>(), *vp, trc, name);
-    return *vp;
+    // Only update *vp if the value changed, to avoid TSan false positives for
+    // template objects when using DumpHeapTracer or UbiNode tracers while Ion
+    // compiling off-thread.
+    Value v = DispatchTyped(DoCallbackFunctor<Value>(), *vp, trc, name);
+    if (*vp != v)
+        *vp = v;
+    return v;
 }
 
 template <>
 jsid
 DoCallback<jsid>(JS::CallbackTracer* trc, jsid* idp, const char* name)
 {
-    *idp = DispatchTyped(DoCallbackFunctor<jsid>(), *idp, trc, name);
-    return *idp;
+    jsid id = DispatchTyped(DoCallbackFunctor<jsid>(), *idp, trc, name);
+    if (*idp != id)
+        *idp = id;
+    return id;
 }
 
 template <>
 TaggedProto
 DoCallback<TaggedProto>(JS::CallbackTracer* trc, TaggedProto* protop, const char* name)
 {
-    *protop = DispatchTyped(DoCallbackFunctor<TaggedProto>(), *protop, trc, name);
-    return *protop;
+    TaggedProto proto = DispatchTyped(DoCallbackFunctor<TaggedProto>(), *protop, trc, name);
+    if (*protop != proto)
+        *protop = proto;
+    return proto;
 }
 
 void
 JS::CallbackTracer::getTracingEdgeName(char* buffer, size_t bufferSize)
 {
     MOZ_ASSERT(bufferSize > 0);
     if (contextFunctor_) {
         (*contextFunctor_)(this, buffer, bufferSize);
--- a/js/src/vm/JSCompartment.cpp
+++ b/js/src/vm/JSCompartment.cpp
@@ -1028,16 +1028,29 @@ JSCompartment::setAllocationMetadataBuil
     // Clear any jitcode in the runtime, which behaves differently depending on
     // whether there is a creation callback.
     ReleaseAllJITCode(runtime_->defaultFreeOp());
 
     allocationMetadataBuilder = builder;
 }
 
 void
+JSCompartment::forgetAllocationMetadataBuilder()
+{
+    // Unlike setAllocationMetadataBuilder, we don't have to discard all JIT
+    // code here (code is still valid, just a bit slower because it doesn't do
+    // inline GC allocations when a metadata builder is present), but we do want
+    // to cancel off-thread Ion compilations to avoid races when Ion calls
+    // hasAllocationMetadataBuilder off-thread.
+    CancelOffThreadIonCompile(this);
+
+    allocationMetadataBuilder = nullptr;
+}
+
+void
 JSCompartment::clearObjectMetadata()
 {
     js_delete(objectMetadataTable);
     objectMetadataTable = nullptr;
 }
 
 void
 JSCompartment::setNewObjectMetadata(JSContext* cx, HandleObject obj)
--- a/js/src/vm/JSCompartment.h
+++ b/js/src/vm/JSCompartment.h
@@ -951,19 +951,17 @@ struct JSCompartment
     void fixupGlobal();
     void fixupScriptMapsAfterMovingGC();
 
     bool hasAllocationMetadataBuilder() const { return allocationMetadataBuilder; }
     const js::AllocationMetadataBuilder* getAllocationMetadataBuilder() const {
         return allocationMetadataBuilder;
     }
     void setAllocationMetadataBuilder(const js::AllocationMetadataBuilder* builder);
-    void forgetAllocationMetadataBuilder() {
-        allocationMetadataBuilder = nullptr;
-    }
+    void forgetAllocationMetadataBuilder();
     void setNewObjectMetadata(JSContext* cx, JS::HandleObject obj);
     void clearObjectMetadata();
     const void* addressOfMetadataBuilder() const {
         return &allocationMetadataBuilder;
     }
 
     js::SavedStacks& savedStacks() { return savedStacks_; }