[webkit-changes] [164627] trunk/Source

Mon, 24 Feb 2014 20:39:06 -0800

Title: [164627] trunk/Source
Revision
164627
Author
[email protected]
Date
2014-02-24 20:44:09 -0800 (Mon, 24 Feb 2014)

Log 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/_javascript_Core: 

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().

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/API/APIShims.h (164626 => 164627)


--- trunk/Source/_javascript_Core/API/APIShims.h	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/_javascript_Core/API/APIShims.h	2014-02-25 04:44:09 UTC (rev 164627)
@@ -58,13 +58,13 @@
 public:
     APIEntryShim(ExecState* exec, bool registerThread = true)
         : APIEntryShimWithoutLock(&exec->vm(), registerThread)
-        , m_lockHolder(exec->vm().exclusiveThread ? 0 : exec)
+        , m_lockHolder(&exec->vm())
     {
     }
 
     APIEntryShim(VM* vm, bool registerThread = true)
         : APIEntryShimWithoutLock(vm, registerThread)
-        , m_lockHolder(vm->exclusiveThread ? 0 : vm)
+        , m_lockHolder(vm)
     {
     }
 
@@ -102,9 +102,6 @@
 private:
     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 
         // released anyways, so it doesn't matter that DropAllLocks is a no-op.

Modified: trunk/Source/_javascript_Core/ChangeLog (164626 => 164627)


--- trunk/Source/_javascript_Core/ChangeLog	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-02-25 04:44:09 UTC (rev 164627)
@@ -1,3 +1,53 @@
+2014-02-24  Mark Lam  <[email protected]>
+
+        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  <[email protected]>
 
         FTL should do polymorphic PutById inlining

Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp (164626 => 164627)


--- trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp	2014-02-25 04:44:09 UTC (rev 164627)
@@ -182,7 +182,7 @@
 
 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))
         return;

Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (164626 => 164627)


--- trunk/Source/_javascript_Core/runtime/JSLock.cpp	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp	2014-02-25 04:44:09 UTC (rev 164627)
@@ -50,7 +50,7 @@
 }
 
 JSLockHolder::JSLockHolder(ExecState* exec)
-    : m_vm(exec ? &exec->vm() : 0)
+    : m_vm(&exec->vm())
 {
     init();
 }
@@ -69,23 +69,21 @@
 
 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();
     apiLock->unlock();
 }
 
 JSLock::JSLock(VM* vm)
-    : m_lockCount(0)
+    : m_ownerThreadID(std::thread::id())
+    , m_lockCount(0)
     , m_lockDropDepth(0)
+    , m_hasExclusiveThread(false)
     , m_vm(vm)
 {
 }
@@ -100,6 +98,13 @@
     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;
+}
+
 void JSLock::lock()
 {
     lock(1);
@@ -113,21 +118,21 @@
         return;
     }
 
-    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;
 
     if (!m_vm)
         return;
 
-    WTFThreadData& threadData = wtfThreadData();
-
     RELEASE_ASSERT(!m_vm->stackPointerAtVMEntry());
     void* p = &p; // A proxy for the current stack pointer.
     m_vm->setStackPointerAtVMEntry(p);
 
+    WTFThreadData& threadData = wtfThreadData();
     m_vm->setLastStackTop(threadData.savedLastStackTop());
 }
 
@@ -146,8 +151,11 @@
     if (!m_lockCount) {
         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();
+        }
     }
 }
 
@@ -163,12 +171,20 @@
 
 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();
 }
 
 // This function returns the number of locks that were dropped.
 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?
     if (!currentThreadIsHoldingLock())
@@ -193,6 +209,8 @@
 
 void JSLock::grabAllLocks(DropAllLocks* dropper, unsigned droppedLockCount)
 {
+    ASSERT(!m_hasExclusiveThread || !droppedLockCount);
+
     // If no locks were dropped, nothing to do!
     if (!droppedLockCount)
         return;

Modified: trunk/Source/_javascript_Core/runtime/JSLock.h (164626 => 164627)


--- trunk/Source/_javascript_Core/runtime/JSLock.h	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/_javascript_Core/runtime/JSLock.h	2014-02-25 04:44:09 UTC (rev 164627)
@@ -94,6 +94,13 @@
 
         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();
 
         void willDestroyVM(VM*);
@@ -129,6 +136,7 @@
         std::thread::id m_ownerThreadID;
         intptr_t m_lockCount;
         unsigned m_lockDropDepth;
+        bool m_hasExclusiveThread;
         VM* m_vm;
     };

Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (164626 => 164627)


--- trunk/Source/_javascript_Core/runtime/VM.cpp	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp	2014-02-25 04:44:09 UTC (rev 164627)
@@ -208,7 +208,6 @@
 #if ENABLE(REGEXP_TRACING)
     , m_rtTraceList(new RTTraceList())
 #endif
-    , exclusiveThread(0)
     , m_newStringsSinceLastHashCons(0)
 #if ENABLE(ASSEMBLER)
     , m_canUseAssembler(enableAssembler(executableAllocator))

Modified: trunk/Source/_javascript_Core/runtime/VM.h (164626 => 164627)


--- trunk/Source/_javascript_Core/runtime/VM.h	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/_javascript_Core/runtime/VM.h	2014-02-25 04:44:09 UTC (rev 164627)
@@ -463,7 +463,9 @@
         RTTraceList* m_rtTraceList;
 #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();
 
@@ -491,10 +493,7 @@
         bool haveEnoughNewStringsToHashCons() { return m_newStringsSinceLastHashCons > s_minNumberOfNewStringsToHashCons; }
         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; }
         CodeCache* codeCache() { return m_codeCache.get(); }

Modified: trunk/Source/WebCore/ChangeLog (164626 => 164627)


--- trunk/Source/WebCore/ChangeLog	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/WebCore/ChangeLog	2014-02-25 04:44:09 UTC (rev 164627)
@@ -1,3 +1,19 @@
+2014-02-24  Mark Lam  <[email protected]>
+
+        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  <[email protected]>
 
         Add a DefaultVisitedLinkProvider and route visited link actions through it

Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp (164626 => 164627)


--- trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp	2014-02-25 04:44:09 UTC (rev 164627)
@@ -153,6 +153,7 @@
 
 void reportException(ExecState* exec, JSValue exception, CachedScript* cachedScript)
 {
+    RELEASE_ASSERT(exec->vm().currentThreadIsHoldingAPILock());
     if (isTerminatedExecutionException(exception))
         return;

Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp (164626 => 164627)


--- trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp	2014-02-25 04:00:13 UTC (rev 164626)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp	2014-02-25 04:44:09 UTC (rev 164627)
@@ -224,7 +224,7 @@
         vm->makeUsableFromMultipleThreads();
         vm->heap.machineThreads().addCurrentThread();
 #else
-        vm->exclusiveThread = currentThread();
+        vm->setExclusiveThread(std::this_thread::get_id());
 #endif // !PLATFORM(IOS)
         initNormalWorldClientData(vm);
     }

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to