Modified: trunk/Source/_javascript_Core/ChangeLog (234064 => 234065)
--- trunk/Source/_javascript_Core/ChangeLog 2018-07-20 21:10:40 UTC (rev 234064)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-07-20 21:14:15 UTC (rev 234065)
@@ -1,3 +1,17 @@
+2018-07-20 Yusuke Suzuki <[email protected]>
+
+ [JSC] Remove cellLock in JSObject::convertContiguousToArrayStorage
+ https://bugs.webkit.org/show_bug.cgi?id=186602
+
+ Reviewed by Saam Barati.
+
+ JSObject::convertContiguousToArrayStorage's cellLock() is not necessary since we do not
+ change the part of the butterfly, length etc. We prove that our procedure is safe, and
+ drop the cellLock() here.
+
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::convertContiguousToArrayStorage):
+
2018-07-20 Saam Barati <[email protected]>
CompareEq should be using KnownOtherUse instead of OtherUse
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (234064 => 234065)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2018-07-20 21:10:40 UTC (rev 234064)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2018-07-20 21:14:15 UTC (rev 234065)
@@ -1341,26 +1341,64 @@
if (v)
newStorage->m_numValuesInVector++;
}
-
+
+ // While we modify the butterfly of Contiguous Array, we do not take any cellLock here. This is because
+ // (1) the old butterfly is not changed and (2) new butterfly is not changed after it is exposed to
+ // the collector.
+ // The mutator performs the following operations are sequentially executed by using storeStoreFence.
+ //
+ // CreateNewButterfly NukeStructure ChangeButterfly PutNewStructure
+ //
+ // Meanwhile the collector performs the following steps sequentially:
+ //
+ // ReadStructureEarly ReadButterfly ReadStructureLate
+ //
+ // We list up all the patterns by writing a tiny script, and ensure all the cases are categorized into BEFORE, AFTER, and IGNORE.
+ //
+ // CreateNewButterfly NukeStructure ChangeButterfly PutNewStructure ReadStructureEarly ReadButterfly ReadStructureLate: AFTER, trivially
+ // CreateNewButterfly NukeStructure ChangeButterfly ReadStructureEarly PutNewStructure ReadButterfly ReadStructureLate: IGNORE, because nuked structure read early
+ // CreateNewButterfly NukeStructure ChangeButterfly ReadStructureEarly ReadButterfly PutNewStructure ReadStructureLate: IGNORE, because nuked structure read early
+ // CreateNewButterfly NukeStructure ChangeButterfly ReadStructureEarly ReadButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read early
+ // CreateNewButterfly NukeStructure ReadStructureEarly ChangeButterfly PutNewStructure ReadButterfly ReadStructureLate: IGNORE, because nuked structure read early
+ // CreateNewButterfly NukeStructure ReadStructureEarly ChangeButterfly ReadButterfly PutNewStructure ReadStructureLate: IGNORE, because nuked structure read early
+ // CreateNewButterfly NukeStructure ReadStructureEarly ChangeButterfly ReadButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read early
+ // CreateNewButterfly NukeStructure ReadStructureEarly ReadButterfly ChangeButterfly PutNewStructure ReadStructureLate: IGNORE, because nuked structure read early
+ // CreateNewButterfly NukeStructure ReadStructureEarly ReadButterfly ChangeButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read early
+ // CreateNewButterfly NukeStructure ReadStructureEarly ReadButterfly ReadStructureLate ChangeButterfly PutNewStructure: IGNORE, because nuked structure read early
+ // CreateNewButterfly ReadStructureEarly NukeStructure ChangeButterfly PutNewStructure ReadButterfly ReadStructureLate: IGNORE, because early and late structures don't match
+ // CreateNewButterfly ReadStructureEarly NukeStructure ChangeButterfly ReadButterfly PutNewStructure ReadStructureLate: IGNORE, because early and late structures don't match
+ // CreateNewButterfly ReadStructureEarly NukeStructure ChangeButterfly ReadButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read late
+ // CreateNewButterfly ReadStructureEarly NukeStructure ReadButterfly ChangeButterfly PutNewStructure ReadStructureLate: IGNORE, because early and late structures don't match
+ // CreateNewButterfly ReadStructureEarly NukeStructure ReadButterfly ChangeButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read late
+ // CreateNewButterfly ReadStructureEarly NukeStructure ReadButterfly ReadStructureLate ChangeButterfly PutNewStructure: IGNORE, because nuked structure read late
+ // CreateNewButterfly ReadStructureEarly ReadButterfly NukeStructure ChangeButterfly PutNewStructure ReadStructureLate: IGNORE, because early and late structures don't match
+ // CreateNewButterfly ReadStructureEarly ReadButterfly NukeStructure ChangeButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read late
+ // CreateNewButterfly ReadStructureEarly ReadButterfly NukeStructure ReadStructureLate ChangeButterfly PutNewStructure: IGNORE, because nuked structure read late
+ // CreateNewButterfly ReadStructureEarly ReadButterfly ReadStructureLate NukeStructure ChangeButterfly PutNewStructure: BEFORE, trivially.
+ // ReadStructureEarly CreateNewButterfly NukeStructure ChangeButterfly PutNewStructure ReadButterfly ReadStructureLate: IGNORE, because early and late structures don't match
+ // ReadStructureEarly CreateNewButterfly NukeStructure ChangeButterfly ReadButterfly PutNewStructure ReadStructureLate: IGNORE, because early and late structures don't match
+ // ReadStructureEarly CreateNewButterfly NukeStructure ChangeButterfly ReadButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read late
+ // ReadStructureEarly CreateNewButterfly NukeStructure ReadButterfly ChangeButterfly PutNewStructure ReadStructureLate: IGNORE, because early and late structures don't match
+ // ReadStructureEarly CreateNewButterfly NukeStructure ReadButterfly ChangeButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read late
+ // ReadStructureEarly CreateNewButterfly NukeStructure ReadButterfly ReadStructureLate ChangeButterfly PutNewStructure: IGNORE, because nuked structure read late
+ // ReadStructureEarly CreateNewButterfly ReadButterfly NukeStructure ChangeButterfly PutNewStructure ReadStructureLate: IGNORE, because early and late structures don't match
+ // ReadStructureEarly CreateNewButterfly ReadButterfly NukeStructure ChangeButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read late
+ // ReadStructureEarly CreateNewButterfly ReadButterfly NukeStructure ReadStructureLate ChangeButterfly PutNewStructure: IGNORE, because nuked structure read late
+ // ReadStructureEarly CreateNewButterfly ReadButterfly ReadStructureLate NukeStructure ChangeButterfly PutNewStructure: BEFORE, CreateNewButterfly is not visible to collector.
+ // ReadStructureEarly ReadButterfly CreateNewButterfly NukeStructure ChangeButterfly PutNewStructure ReadStructureLate: IGNORE, because early and late structures don't match
+ // ReadStructureEarly ReadButterfly CreateNewButterfly NukeStructure ChangeButterfly ReadStructureLate PutNewStructure: IGNORE, because nuked structure read late
+ // ReadStructureEarly ReadButterfly CreateNewButterfly NukeStructure ReadStructureLate ChangeButterfly PutNewStructure: IGNORE, because nuked structure read late
+ // ReadStructureEarly ReadButterfly CreateNewButterfly ReadStructureLate NukeStructure ChangeButterfly PutNewStructure: BEFORE, CreateNewButterfly is not visible to collector.
+ // ReadStructureEarly ReadButterfly ReadStructureLate CreateNewButterfly NukeStructure ChangeButterfly PutNewStructure: BEFORE, trivially.
+
+ ASSERT(newStorage->butterfly() != butterfly);
StructureID oldStructureID = this->structureID();
Structure* oldStructure = vm.getStructure(oldStructureID);
Structure* newStructure = Structure::nonPropertyTransition(vm, oldStructure, transition);
- // This has a crazy race with the garbage collector. When changing the butterfly and structure,
- // the mutator always sets the structure last. The collector will always read the structure
- // first. We probably have to follow that convention here as well. This means that the collector
- // will sometimes see the new butterfly (the one with ArrayStorage) with the only structure (the
- // one that says that the butterfly is Contiguous). When scanning Contiguous, the collector will
- // mark word at addresses greater than or equal to the butterfly pointer, up to the publicLength
- // in the butterfly. But an ArrayStorage has some non-pointer header data at low positive
- // offsets from the butterfly - so when this race happens, the collector will surely crash
- // because it will fail to decode two consecutive int32s as if it was a JSValue.
- //
- // Fortunately, we have the JSCell lock for this purpose!
-
- Locker<JSCellLock> locker(NoLockingNecessary);
- if (vm.heap.mutatorShouldBeFenced())
- locker = holdLock(cellLock());
+ // Ensure new Butterfly initialization is correctly done before exposing it to the concurrent threads.
+ if (isX86() || vm.heap.mutatorShouldBeFenced())
+ WTF::storeStoreFence();
nukeStructureAndSetButterfly(vm, oldStructureID, newStorage->butterfly());
setStructure(vm, newStructure);
