Changeset 164627 in webkit

Timestamp:

Feb 24, 2014 8:44:09 PM (10 years ago)

Author:

mark.lam@apple.com

Message:

Need to initialize VM stack data even when the VM is on an exclusive thread.
<​https://webkit.org/b/129265>

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

We check VM::exclusiveThread as an optimization to forego the need to do
JSLock locking. However, we recently started piggy backing on JSLock's
lock() and unlock() to initialize VM stack data (stackPointerAtVMEntry
and lastStackTop) to appropriate values for the current thread. This is
needed because we may be acquiring the lock to enter the VM on a different
thread.

As a result, we ended up not initializing the VM stack data when
VM::exclusiveThread causes us to bypass the locking activity. Even though
the VM::exclusiveThread will not have to deal with the VM being entered
on a different thread, it still needs to initialize the VM stack data.
The VM relies on that data being initialized properly once it has been
entered.

With this fix, we push the check for exclusiveThread down into the JSLock,
and handle the bypassing of unneeded locking activity there while still
executing the necessary the VM stack data initialization.

• API/APIShims.h:

(JSC::APIEntryShim::APIEntryShim):
(JSC::APICallbackShim::shouldDropAllLocks):

• heap/MachineStackMarker.cpp:

(JSC::MachineThreads::addCurrentThread):

• runtime/JSLock.cpp:

(JSC::JSLockHolder::JSLockHolder):
(JSC::JSLockHolder::init):
(JSC::JSLockHolder::~JSLockHolder):
(JSC::JSLock::JSLock):
(JSC::JSLock::setExclusiveThread):
(JSC::JSLock::lock):
(JSLock::unlock):
(JSLock::currentThreadIsHoldingLock):
(JSLock::dropAllLocks):
(JSLock::grabAllLocks):

• runtime/JSLock.h:

(JSC::JSLock::exclusiveThread):

• runtime/VM.cpp:

(JSC::VM::VM):

• runtime/VM.h:

(JSC::VM::exclusiveThread):
(JSC::VM::setExclusiveThread):
(JSC::VM::currentThreadIsHoldingAPILock):

Source/WebCore:

No new tests.

• bindings/js/JSDOMBinding.cpp:

(WebCore::reportException):

• Added an assertion to ensure that we are holding the JSLock.
• bindings/js/JSDOMWindowBase.cpp:

(WebCore::JSDOMWindowBase::commonVM):

• Updated to use the new VM::setExclusiveThread().

Location:

trunk/Source

Files:

• JavaScriptCore/API/APIShims.h (modified) (3 diffs)
• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/heap/MachineStackMarker.cpp (modified) (1 diff)
• JavaScriptCore/runtime/JSLock.cpp (modified) (11 diffs)
• JavaScriptCore/runtime/JSLock.h (modified) (2 diffs)
• JavaScriptCore/runtime/VM.cpp (modified) (1 diff)
• JavaScriptCore/runtime/VM.h (modified) (2 diffs)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/JSDOMBinding.cpp (modified) (1 diff)
• WebCore/bindings/js/JSDOMWindowBase.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/API/APIShims.h

@@ -59 +59 @@
     APIEntryShim(ExecState* exec, bool registerThread = true)
         : APIEntryShimWithoutLock(&exec->vm(), registerThread)
-        , m_lockHolder(exec->vm().exclusiveThread ? 0 : exec)
+        , m_lockHolder(&exec->vm())
     {
     }
@@ -65 +65 @@
     APIEntryShim(VM* vm, bool registerThread = true)
         : APIEntryShimWithoutLock(vm, registerThread)
-        , m_lockHolder(vm->exclusiveThread ? 0 : vm)
+        , m_lockHolder(vm)
     {
     }
@@ -103 +103 @@
     static bool shouldDropAllLocks(VM& vm)
     {
-        if (vm.exclusiveThread)
-            return false;
-
         // If the VM is in the middle of being destroyed then we don't want to resurrect it
         // by allowing DropAllLocks to ref it. By this point the APILock has already been 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2014-02-24  Mark Lam  <mark.lam@apple.com>
+
+        Need to initialize VM stack data even when the VM is on an exclusive thread.
+        <https://webkit.org/b/129265>
+
+        Reviewed by Geoffrey Garen.
+
+        We check VM::exclusiveThread as an optimization to forego the need to do
+        JSLock locking. However, we recently started piggy backing on JSLock's
+        lock() and unlock() to initialize VM stack data (stackPointerAtVMEntry
+        and lastStackTop) to appropriate values for the current thread. This is
+        needed because we may be acquiring the lock to enter the VM on a different
+        thread.
+
+        As a result, we ended up not initializing the VM stack data when
+        VM::exclusiveThread causes us to bypass the locking activity. Even though
+        the VM::exclusiveThread will not have to deal with the VM being entered
+        on a different thread, it still needs to initialize the VM stack data.
+        The VM relies on that data being initialized properly once it has been
+        entered.
+
+        With this fix, we push the check for exclusiveThread down into the JSLock,
+        and handle the bypassing of unneeded locking activity there while still
+        executing the necessary the VM stack data initialization.
+
+        * API/APIShims.h:
+        (JSC::APIEntryShim::APIEntryShim):
+        (JSC::APICallbackShim::shouldDropAllLocks):
+        * heap/MachineStackMarker.cpp:
+        (JSC::MachineThreads::addCurrentThread):
+        * runtime/JSLock.cpp:
+        (JSC::JSLockHolder::JSLockHolder):
+        (JSC::JSLockHolder::init):
+        (JSC::JSLockHolder::~JSLockHolder):
+        (JSC::JSLock::JSLock):
+        (JSC::JSLock::setExclusiveThread):
+        (JSC::JSLock::lock):
+        (JSLock::unlock):
+        (JSLock::currentThreadIsHoldingLock):
+        (JSLock::dropAllLocks):
+        (JSLock::grabAllLocks):
+        * runtime/JSLock.h:
+        (JSC::JSLock::exclusiveThread):
+        * runtime/VM.cpp:
+        (JSC::VM::VM):
+        * runtime/VM.h:
+        (JSC::VM::exclusiveThread):
+        (JSC::VM::setExclusiveThread):
+        (JSC::VM::currentThreadIsHoldingAPILock):
+
 2014-02-24  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp

@@ -183 +183 @@
 void MachineThreads::addCurrentThread()
 {
-    ASSERT(!m_heap->vm()->exclusiveThread || m_heap->vm()->exclusiveThread == currentThread());
+    ASSERT(!m_heap->vm()->hasExclusiveThread() || m_heap->vm()->exclusiveThread() == std::this_thread::get_id());
 
     if (!m_threadSpecific || threadSpecificGet(m_threadSpecific))

trunk/Source/JavaScriptCore/runtime/JSLock.cpp

@@ -51 +51 @@
 
 JSLockHolder::JSLockHolder(ExecState* exec)
-    : m_vm(exec ? &exec->vm() : 0)
+    : m_vm(&exec->vm())
 {
     init();
@@ -70 +70 @@
 void JSLockHolder::init()
 {
-    if (m_vm)
-        m_vm->apiLock().lock();
+    m_vm->apiLock().lock();
 }
 
 JSLockHolder::~JSLockHolder()
 {
-    if (!m_vm)
-        return;
-
     RefPtr<JSLock> apiLock(&m_vm->apiLock());
     m_vm.clear();
@@ -85 +81 @@
 
 JSLock::JSLock(VM* vm)
-    : m_lockCount(0)
+    : m_ownerThreadID(std::thread::id())
+    , m_lockCount(0)
     , m_lockDropDepth(0)
+    , m_hasExclusiveThread(false)
     , m_vm(vm)
 {
@@ -99 +97 @@
     ASSERT_UNUSED(vm, m_vm == vm);
     m_vm = 0;
+}
+
+void JSLock::setExclusiveThread(std::thread::id threadId)
+{
+    RELEASE_ASSERT(!m_lockCount && m_ownerThreadID == std::thread::id());
+    m_hasExclusiveThread = (threadId != std::thread::id());
+    m_ownerThreadID = threadId;
 }
 
@@ -114 +119 @@
     }
 
-    m_lock.lock();
-
-    m_ownerThreadID = std::this_thread::get_id();
+    if (!m_hasExclusiveThread) {
+        m_lock.lock();
+        m_ownerThreadID = std::this_thread::get_id();
+    }
     ASSERT(!m_lockCount);
     m_lockCount = lockCount;
@@ -122 +128 @@
     if (!m_vm)
         return;
-
-    WTFThreadData& threadData = wtfThreadData();
 
     RELEASE_ASSERT(!m_vm->stackPointerAtVMEntry());
@@ -129 +133 @@
     m_vm->setStackPointerAtVMEntry(p);
 
+    WTFThreadData& threadData = wtfThreadData();
     m_vm->setLastStackTop(threadData.savedLastStackTop());
 }
@@ -147 +152 @@
         if (m_vm)
             m_vm->setStackPointerAtVMEntry(nullptr);
-        m_ownerThreadID = std::thread::id();
-        m_lock.unlock();
+
+        if (!m_hasExclusiveThread) {
+            m_ownerThreadID = std::thread::id();
+            m_lock.unlock();
+        }
     }
 }
@@ -164 +172 @@
 bool JSLock::currentThreadIsHoldingLock()
 {
+    ASSERT(!m_hasExclusiveThread || (exclusiveThread() == std::this_thread::get_id()));
+    if (m_hasExclusiveThread)
+        return !!m_lockCount;
     return m_ownerThreadID == std::this_thread::get_id();
 }
@@ -170 +181 @@
 unsigned JSLock::dropAllLocks(DropAllLocks* dropper)
 {
+    if (m_hasExclusiveThread) {
+        ASSERT(exclusiveThread() == std::this_thread::get_id());
+        return 0;
+    }
+
     // Check if this thread is currently holding the lock.
     // FIXME: Maybe we want to require this, guard with an ASSERT?
@@ -194 +210 @@
 void JSLock::grabAllLocks(DropAllLocks* dropper, unsigned droppedLockCount)
 {
+    ASSERT(!m_hasExclusiveThread || !droppedLockCount);
+
     // If no locks were dropped, nothing to do!
     if (!droppedLockCount)

trunk/Source/JavaScriptCore/runtime/JSLock.h

@@ -95 +95 @@
         VM* vm() { return m_vm; }
 
+        bool hasExclusiveThread() const { return m_hasExclusiveThread; }
+        std::thread::id exclusiveThread() const
+        {
+            ASSERT(m_hasExclusiveThread);
+            return m_ownerThreadID;
+        }
+        JS_EXPORT_PRIVATE void setExclusiveThread(std::thread::id);
         JS_EXPORT_PRIVATE bool currentThreadIsHoldingLock();
 
@@ -130 +137 @@
         intptr_t m_lockCount;
         unsigned m_lockDropDepth;
+        bool m_hasExclusiveThread;
         VM* m_vm;
     };

trunk/Source/JavaScriptCore/runtime/VM.cpp

@@ -209 +209 @@
     , m_rtTraceList(new RTTraceList())
 #endif
-    , exclusiveThread(0)
     , m_newStringsSinceLastHashCons(0)
 #if ENABLE(ASSEMBLER)

trunk/Source/JavaScriptCore/runtime/VM.h

@@ -464 +464 @@
 #endif
 
-        ThreadIdentifier exclusiveThread;
+        bool hasExclusiveThread() const { return m_apiLock->hasExclusiveThread(); }
+        std::thread::id exclusiveThread() const { return m_apiLock->exclusiveThread(); }
+        void setExclusiveThread(std::thread::id threadId) { m_apiLock->setExclusiveThread(threadId); }
 
         JS_EXPORT_PRIVATE void resetDateCache();
@@ -492 +494 @@
         void resetNewStringsSinceLastHashCons() { m_newStringsSinceLastHashCons = 0; }
 
-        bool currentThreadIsHoldingAPILock() const
-        {
-            return m_apiLock->currentThreadIsHoldingLock() || exclusiveThread == currentThread();
-        }
+        bool currentThreadIsHoldingAPILock() const { return m_apiLock->currentThreadIsHoldingLock(); }
 
         JSLock& apiLock() { return *m_apiLock; }

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-02-24  Mark Lam  <mark.lam@apple.com>
+
+        Need to initialize VM stack data even when the VM is on an exclusive thread.
+        <https://webkit.org/b/129265>
+
+        Reviewed by Geoffrey Garen.
+
+        No new tests.
+
+        * bindings/js/JSDOMBinding.cpp:
+        (WebCore::reportException):
+        - Added an assertion to ensure that we are holding the JSLock.
+        * bindings/js/JSDOMWindowBase.cpp:
+        (WebCore::JSDOMWindowBase::commonVM):
+        - Updated to use the new VM::setExclusiveThread().
+
 2014-02-24  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp

@@ -154 +154 @@
 void reportException(ExecState* exec, JSValue exception, CachedScript* cachedScript)
 {
+    RELEASE_ASSERT(exec->vm().currentThreadIsHoldingAPILock());
     if (isTerminatedExecutionException(exception))
         return;

trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp

@@ -225 +225 @@
         vm->heap.machineThreads().addCurrentThread();
 #else
-        vm->exclusiveThread = currentThread();
+        vm->setExclusiveThread(std::this_thread::get_id());
 #endif // !PLATFORM(IOS)
         initNormalWorldClientData(vm);