Changeset 157539 in webkit for trunk/Source/JavaScriptCore/runtime/Structure.h

Timestamp:

Oct 16, 2013, 4:47:45 PM (12 years ago)

Author:

[email protected]

Message:

llint_slow_path_put_by_id can deadlock on a ConcurrentJITLock
​https://bugs.webkit.org/show_bug.cgi?id=122667

Reviewed by Geoffrey Garen.

The issue this patch is attempting to fix is that there are places in our codebase
where we acquire the ConcurrentJITLock for a particular CodeBlock, then we do some
operations that can initiate a garbage collection. Garbage collection then calls
some methods of CodeBlock that also take the ConcurrentJITLock (because they don't
always necessarily run during garbage collection). This causes a deadlock.

To fix this issue, this patch adds a new RAII-style object (DisallowGC) that stores
into a thread-local field that indicates that it is unsafe to perform any operation
that could trigger garbage collection on the current thread. In debug builds,
ConcurrentJITLocker contains one of these DisallowGC objects so that we can eagerly
detect deadlocks.

This patch also adds a new type of ConcurrentJITLocker, GCSafeConcurrentJITLocker,
which uses the DeferGC mechanism to prevent collections from occurring while the
lock is held.

• CMakeLists.txt:
• GNUmakefile.list.am:
• JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
• JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:
• JavaScriptCore.xcodeproj/project.pbxproj:
• heap/DeferGC.h:

(JSC::DisallowGC::DisallowGC):
(JSC::DisallowGC::~DisallowGC):
(JSC::DisallowGC::isGCDisallowedOnCurrentThread):
(JSC::DisallowGC::initialize):

• jit/Repatch.cpp:

(JSC::repatchPutByID):
(JSC::buildPutByIdList):

• llint/LLIntSlowPaths.cpp:

(JSC::LLInt::LLINT_SLOW_PATH_DECL):

• runtime/ConcurrentJITLock.h:

(JSC::ConcurrentJITLockerBase::ConcurrentJITLockerBase):
(JSC::ConcurrentJITLockerBase::~ConcurrentJITLockerBase):
(JSC::ConcurrentJITLockerBase::unlockEarly):
(JSC::GCSafeConcurrentJITLocker::GCSafeConcurrentJITLocker):
(JSC::GCSafeConcurrentJITLocker::~GCSafeConcurrentJITLocker):
(JSC::GCSafeConcurrentJITLocker::NoDefer::NoDefer):
(JSC::ConcurrentJITLocker::ConcurrentJITLocker):

• runtime/InitializeThreading.cpp:

(JSC::initializeThreadingOnce):

• runtime/JSCellInlines.h:

(JSC::allocateCell):

• runtime/JSSymbolTableObject.h:

(JSC::symbolTablePut):

• runtime/Structure.cpp: materializePropertyMapIfNecessary* now has a problem in that it

can start a garbage collection when the GCSafeConcurrentJITLocker goes out of scope, but
before the caller has a chance to use the newly created PropertyTable. The garbage collection
clears the PropertyTable, and then the caller uses it assuming it's valid. To avoid this,
we must DeferGC until the caller is done getting the newly materialized PropertyTable from
the Structure.
(JSC::Structure::materializePropertyMap):
(JSC::Structure::despecifyDictionaryFunction):
(JSC::Structure::changePrototypeTransition):
(JSC::Structure::despecifyFunctionTransition):
(JSC::Structure::attributeChangeTransition):
(JSC::Structure::toDictionaryTransition):
(JSC::Structure::preventExtensionsTransition):
(JSC::Structure::takePropertyTableOrCloneIfPinned):
(JSC::Structure::isSealed):
(JSC::Structure::isFrozen):
(JSC::Structure::addPropertyWithoutTransition):
(JSC::Structure::removePropertyWithoutTransition):
(JSC::Structure::get):
(JSC::Structure::despecifyFunction):
(JSC::Structure::despecifyAllFunctions):
(JSC::Structure::putSpecificValue):
(JSC::Structure::createPropertyMap):
(JSC::Structure::getPropertyNamesFromStructure):

• runtime/Structure.h:

(JSC::Structure::materializePropertyMapIfNecessary):
(JSC::Structure::materializePropertyMapIfNecessaryForPinning):

• runtime/StructureInlines.h:

(JSC::Structure::get):

• runtime/SymbolTable.h:

(JSC::SymbolTable::find):
(JSC::SymbolTable::end):

File:

• trunk/Source/JavaScriptCore/runtime/Structure.h (modified) (4 diffs)

trunk/Source/JavaScriptCore/runtime/Structure.h

@@ -52 +52 @@
 namespace JSC {
 
+class DeferGC;
 class LLIntOffsetsExtractor;
 class PropertyNameArray;
@@ -394 +395 @@
     PropertyOffset remove(PropertyName);
 
-    void createPropertyMap(const ConcurrentJITLocker&, VM&, unsigned keyCount = 0);
+    void createPropertyMap(const GCSafeConcurrentJITLocker&, VM&, unsigned keyCount = 0);
     void checkConsistency();
 
@@ -405 +406 @@
     PropertyTable* copyPropertyTableForPinning(VM&, Structure* owner);
     JS_EXPORT_PRIVATE void materializePropertyMap(VM&);
-    void materializePropertyMapIfNecessary(VM& vm)
+    void materializePropertyMapIfNecessary(VM& vm, DeferGC&)
     {
         ASSERT(!isCompilationThread());
@@ -413 +414 @@
             materializePropertyMap(vm);
     }
-    void materializePropertyMapIfNecessaryForPinning(VM& vm)
+    void materializePropertyMapIfNecessaryForPinning(VM& vm, DeferGC&)
     {
         ASSERT(structure()->classInfo() == info());