Changeset 208953 in webkit for trunk/Source/JavaScriptCore/runtime/TemplateRegistry.cpp

Timestamp:

Nov 21, 2016, 3:54:43 PM (9 years ago)

Author:

Yusuke Suzuki

Message:

Crash in com.apple.JavaScriptCore: WTF::ThreadSpecific<WTF::WTFThreadData, + 142
​https://bugs.webkit.org/show_bug.cgi?id=164898

Reviewed by Darin Adler.

JSTests:

• stress/tagged-template-registry-key-collect.js: Added.

(shouldBe):
(tag):
(i.eval):

• stress/tagged-template-registry-key.js: Added.

(shouldBe):
(tag):
(a):
(b):

Source/JavaScriptCore:

The callsite object (JSArray) of tagged template literal is managed by WeakGCMap since
same tagged template literal need to return an identical object.
The problem is that we used TemplateRegistryKey as the key of the WeakGCMap. WeakGCMap
can prune its entries in the collector thread. At that time, this TemplateRegistryKey
is deallocated. Since it includes String (and then, StringImpl), we accidentally call
ref(), deref() and StringImpl::destroy() in the different thread from the main thread
while this TemplateRegistryKey is allocated in the main thread.

Instead, we use TemplateRegistryKey* as the key of WeakGCMap. Then, to keep its liveness
while the entry of the WeakGCMap is alive, the callsite object has the reference to
the JSTemplateRegistryKey. And it holds Ref<TemplateRegistryKey>.

And now we need to lookup WeakGCMap with TemplateRegistryKey*. To do so, we create
interning system for TemplateRegistryKey. It is similar to AtomicStringTable and
SymbolRegistry. TemplateRegistryKey is allocated from this table. This table atomize the
TemplateRegistryKey. So we can use the pointer comparison between TemplateRegistryKey.
It allows us to lookup the entry from WeakGCMap by TemplateRegistryKey*.

• CMakeLists.txt:
• JavaScriptCore.xcodeproj/project.pbxproj:
• builtins/BuiltinNames.h:
• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::addTemplateRegistryKeyConstant):
(JSC::BytecodeGenerator::emitGetTemplateObject):

• bytecompiler/BytecodeGenerator.h:
• runtime/JSGlobalObject.cpp:

(JSC::getTemplateObject):

• runtime/JSTemplateRegistryKey.cpp:

(JSC::JSTemplateRegistryKey::JSTemplateRegistryKey):
(JSC::JSTemplateRegistryKey::create):

• runtime/JSTemplateRegistryKey.h:
• runtime/TemplateRegistry.cpp:

(JSC::TemplateRegistry::getTemplateObject):

• runtime/TemplateRegistry.h:
• runtime/TemplateRegistryKey.cpp: Copied from Source/JavaScriptCore/runtime/TemplateRegistry.h.

(JSC::TemplateRegistryKey::~TemplateRegistryKey):

• runtime/TemplateRegistryKey.h:

(JSC::TemplateRegistryKey::calculateHash):
(JSC::TemplateRegistryKey::create):
(JSC::TemplateRegistryKey::TemplateRegistryKey):

• runtime/TemplateRegistryKeyTable.cpp: Added.

(JSC::TemplateRegistryKeyTranslator::hash):
(JSC::TemplateRegistryKeyTranslator::equal):
(JSC::TemplateRegistryKeyTranslator::translate):
(JSC::TemplateRegistryKeyTable::~TemplateRegistryKeyTable):
(JSC::TemplateRegistryKeyTable::createKey):
(JSC::TemplateRegistryKeyTable::unregister):

• runtime/TemplateRegistryKeyTable.h: Copied from Source/JavaScriptCore/runtime/JSTemplateRegistryKey.h.

(JSC::TemplateRegistryKeyTable::KeyHash::hash):
(JSC::TemplateRegistryKeyTable::KeyHash::equal):

• runtime/VM.h:

(JSC::VM::templateRegistryKeyTable):

File:

• trunk/Source/JavaScriptCore/runtime/TemplateRegistry.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/runtime/TemplateRegistry.cpp

@@ -28 +28 @@
 #include "TemplateRegistry.h"
 
+#include "BuiltinNames.h"
 #include "JSCInlines.h"
 #include "JSGlobalObject.h"
+#include "JSTemplateRegistryKey.h"
 #include "ObjectConstructor.h"
+#include "TemplateRegistryKey.h"
 #include "WeakGCMapInlines.h"
 
@@ -40 +43 @@
 }
 
-JSArray* TemplateRegistry::getTemplateObject(ExecState* exec, const TemplateRegistryKey& templateKey)
+JSArray* TemplateRegistry::getTemplateObject(ExecState* exec, JSTemplateRegistryKey* templateKeyObject)
 {
-    JSArray* cached = m_templateMap.get(templateKey);
+    auto& templateKey = templateKeyObject->templateRegistryKey();
+    JSArray* cached = m_templateMap.get(&templateKey);
     if (cached)
         return cached;
@@ -64 +68 @@
     templateObject->putDirect(vm, exec->propertyNames().raw, rawObject, ReadOnly | DontEnum | DontDelete);
 
+    // Template JSArray hold the reference to JSTemplateRegistryKey to make TemplateRegistryKey pointer live until this JSArray is collected.
+    // TemplateRegistryKey pointer is used for TemplateRegistry's key.
+    templateObject->putDirect(vm, vm.propertyNames->builtinNames().templateRegistryKeyPrivateName(), templateKeyObject, ReadOnly | DontEnum | DontDelete);
+
     objectConstructorFreeze(exec, templateObject);
     ASSERT(!scope.exception());
 
-    m_templateMap.set(templateKey, templateObject);
+    m_templateMap.set(&templateKey, templateObject);
 
     return templateObject;