Changeset 157413 in webkit for trunk/Source/JavaScriptCore/runtime/ConcurrentJITLock.h

Timestamp:

Oct 14, 2013, 12:34:44 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 Filip Pizlo.

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.cpp: Added.
• heap/DeferGC.h:

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

• jit/JITStubs.cpp:

(JSC::tryCachePutByID):
(JSC::tryCacheGetByID):
(JSC::DEFINE_STUB_FUNCTION):

• 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::ConcurrentJITLocker::ConcurrentJITLocker):

• runtime/InitializeThreading.cpp:

(JSC::initializeThreadingOnce):

• runtime/JSCellInlines.h:

(JSC::allocateCell):

• runtime/Structure.cpp:

(JSC::Structure::materializePropertyMap):
(JSC::Structure::putSpecificValue):
(JSC::Structure::createPropertyMap):

• runtime/Structure.h:

File:

• trunk/Source/JavaScriptCore/runtime/ConcurrentJITLock.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/runtime/ConcurrentJITLock.h

@@ -27 +27 @@
 #define ConcurrentJITLock_h
 
+#include "DeferGC.h"
 #include <wtf/ByteSpinLock.h>
 #include <wtf/NoLock.h>
@@ -34 +35 @@
 #if ENABLE(CONCURRENT_JIT)
 typedef ByteSpinLock ConcurrentJITLock;
-typedef ByteSpinLocker ConcurrentJITLocker;
+typedef ByteSpinLocker ConcurrentJITLockerImpl;
 #else
 typedef NoLock ConcurrentJITLock;
-typedef NoLockLocker ConcurrentJITLocker;
+typedef NoLockLocker ConcurrentJITLockerImpl;
 #endif
+
+class ConcurrentJITLockerBase {
+    WTF_MAKE_NONCOPYABLE(ConcurrentJITLockerBase);
+public:
+    explicit ConcurrentJITLockerBase(ConcurrentJITLock& lockable)
+        : m_locker(&lockable)
+    {
+    }
+    explicit ConcurrentJITLockerBase(ConcurrentJITLock* lockable)
+        : m_locker(lockable)
+    {
+    }
+
+    ~ConcurrentJITLockerBase()
+    {
+    }
+    
+    void unlockEarly()
+    {
+        m_locker.unlockEarly();
+    }
+
+private:
+    ConcurrentJITLockerImpl m_locker;
+};
+
+class GCSafeConcurrentJITLocker : public ConcurrentJITLockerBase {
+public:
+    GCSafeConcurrentJITLocker(ConcurrentJITLock& lockable, Heap& heap)
+        : ConcurrentJITLockerBase(lockable)
+        , m_deferGC(heap)
+    {
+    }
+
+    GCSafeConcurrentJITLocker(ConcurrentJITLock* lockable, Heap& heap)
+        : ConcurrentJITLockerBase(lockable)
+        , m_deferGC(heap)
+    {
+    }
+
+private:
+#if ENABLE(CONCURRENT_JIT)
+    DeferGC m_deferGC;
+#else
+    struct NoDefer {
+        NoDefer(Heap& heap) : m_heap(heap) { }
+        Heap& m_heap;
+    };
+    NoDefer m_deferGC;
+#endif
+};
+
+class ConcurrentJITLocker : public ConcurrentJITLockerBase {
+public:
+    ConcurrentJITLocker(ConcurrentJITLock& lockable)
+        : ConcurrentJITLockerBase(lockable)
+    {
+    }
+
+    ConcurrentJITLocker(ConcurrentJITLock* lockable)
+        : ConcurrentJITLockerBase(lockable)
+    {
+    }
+
+#if ENABLE(CONCURRENT_JIT) && !defined(NDEBUG)
+private:
+    DisallowGC m_disallowGC;
+#endif
+};
 
 } // namespace JSC
 
 #endif // ConcurrentJITLock_h
-