Changeset 163855 in webkit


Ignore:
Timestamp:
Feb 10, 2014 8:48:01 PM (10 years ago)
Author:
mark.lam@apple.com
Message:

Source/JavaScriptCore: Removing limitation on JSLock's lockDropDepth.
<​https://webkit.org/b/128570>

Reviewed by Geoffrey Garen.

Now that we've switched to using the C stack, we no longer need to limit
the JSLock::lockDropDepth to 2.

For C loop builds which still use the separate JSStack, the JSLock will
enforce ordering for re-grabbing the lock after dropping it. Re-grabbing
must occur in the reverse order of the dropping of the locks.

Ordering is achieved by JSLock::dropAllLocks() stashing away the
JSLock:: m_lockDropDepth in its DropAllLocks instance's m_dropDepth
before unlocking the lock. Subsequently, JSLock::grabAllLocks() will
ensure that JSLocks::m_lockDropDepth equals its DropAllLocks instance's
m_dropDepth before allowing the lock to be re-grabbed. Otherwise, it
will yield execution and retry again later.

Note: because JSLocks::m_lockDropDepth is protected by the JSLock's
mutex, grabAllLocks() will optimistically lock the JSLock before doing
the check on m_lockDropDepth. If the check fails, it will unlock the
JSLock, yield, and then relock it again later before retrying the check.
This ensures that m_lockDropDepth remains under the protection of the
JSLock's mutex.

  • runtime/JSLock.cpp:

(JSC::JSLock::dropAllLocks):
(JSC::JSLock::grabAllLocks):
(JSC::JSLock::DropAllLocks::DropAllLocks):
(JSC::JSLock::DropAllLocks::~DropAllLocks):

  • runtime/JSLock.h:

(JSC::JSLock::DropAllLocks::setDropDepth):
(JSC::JSLock::DropAllLocks::dropDepth):

Source/WebCore: Removing limitation on JSLock’s lockDropDepth.
<​https://webkit.org/b/128570>

Reviewed by Geoffrey Garen.

No new tests.

  • bindings/js/PageScriptDebugServer.cpp:

(WebCore::PageScriptDebugServer::runEventLoopWhilePaused):

  • platform/ios/wak/WebCoreThread.mm:

(SendDelegateMessage):
(WebThreadRunOnMainThread):

  • No longer need to specify AlwaysDropLocks, because DropAllLocks is now always unconditional.
Location:
trunk/Source
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r163851 r163855  
     12014-02-10  Mark Lam  <mark.lam@apple.com>
     2
     3        Removing limitation on JSLock's lockDropDepth.
     4        <https://webkit.org/b/128570>
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Now that we've switched to using the C stack, we no longer need to limit
     9        the JSLock::lockDropDepth to 2.
     10
     11        For C loop builds which still use the separate JSStack, the JSLock will
     12        enforce ordering for re-grabbing the lock after dropping it. Re-grabbing
     13        must occur in the reverse order of the dropping of the locks.
     14
     15        Ordering is achieved by JSLock::dropAllLocks() stashing away the
     16        JSLock:: m_lockDropDepth in its DropAllLocks instance's m_dropDepth
     17        before unlocking the lock. Subsequently, JSLock::grabAllLocks() will
     18        ensure that JSLocks::m_lockDropDepth equals its DropAllLocks instance's
     19        m_dropDepth before allowing the lock to be re-grabbed. Otherwise, it
     20        will yield execution and retry again later.
     21
     22        Note: because JSLocks::m_lockDropDepth is protected by the JSLock's
     23        mutex, grabAllLocks() will optimistically lock the JSLock before doing
     24        the check on m_lockDropDepth. If the check fails, it will unlock the
     25        JSLock, yield, and then relock it again later before retrying the check.
     26        This ensures that m_lockDropDepth remains under the protection of the
     27        JSLock's mutex.
     28
     29        * runtime/JSLock.cpp:
     30        (JSC::JSLock::dropAllLocks):
     31        (JSC::JSLock::grabAllLocks):
     32        (JSC::JSLock::DropAllLocks::DropAllLocks):
     33        (JSC::JSLock::DropAllLocks::~DropAllLocks):
     34        * runtime/JSLock.h:
     35        (JSC::JSLock::DropAllLocks::setDropDepth):
     36        (JSC::JSLock::DropAllLocks::dropDepth):
     37
    1382014-02-10  Filip Pizlo  <fpizlo@apple.com>
    239

  • trunk/Source/JavaScriptCore/runtime/JSLock.cpp

    r163844 r163855  
    2727#include "JSObject.h"
    2828#include "JSCInlines.h"
     29#if ENABLE(LLINT_C_LOOP)
     30#include <thread>
     31#endif
    2932
    3033namespace JSC {
    … …  
    167170}
    168171
    169 // This is fairly nasty.  We allow multiple threads to run on the same
    170 // context, and we do not require any locking semantics in doing so -
    171 // clients of the API may simply use the context from multiple threads
    172 // concurently, and assume this will work.  In order to make this work,
    173 // We lock the context when a thread enters, and unlock it when it leaves.
    174 // However we do not only unlock when the thread returns from its
    175 // entry point (evaluate script or call function), we also unlock the
    176 // context if the thread leaves JSC by making a call out to an external
    177 // function through a callback.
    178 //
    179 // All threads using the context share the same JS stack (the JSStack).
    180 // Whenever a thread calls into JSC it starts using the JSStack from the
    181 // previous 'high water mark' - the maximum point the stack has ever grown to
    182 // (returned by JSStack::end()).  So if a first thread calls out to a
    183 // callback, and a second thread enters JSC, then also exits by calling out
    184 // to a callback, we can be left with stackframes from both threads in the
    185 // JSStack.  As such, a problem may occur should the first thread's
    186 // callback complete first, and attempt to return to JSC.  Were we to allow
    187 // this to happen, and were its stack to grow further, then it may potentially
    188 // write over the second thread's call frames.
    189 //
    190 // To avoid JS stack corruption we enforce a policy of only ever allowing two
    191 // threads to use a JS context concurrently, and only allowing the second of
    192 // these threads to execute until it has completed and fully returned from its
    193 // outermost call into JSC.  We enforce this policy using 'lockDropDepth'.  The
    194 // first time a thread exits it will call DropAllLocks - which will do as expected
    195 // and drop locks allowing another thread to enter.  Should another thread, or the
    196 // same thread again, enter JSC (through evaluate script or call function), and exit
    197 // again through a callback, then the locks will not be dropped when DropAllLocks
    198 // is called (since lockDropDepth is non-zero).  Since this thread is still holding
    199 // the locks, only it will be able to re-enter JSC (either be returning from the
    200 // callback, or by re-entering through another call to evaulate script or call
    201 // function).
    202 //
    203 // This policy is slightly more restricive than it needs to be for correctness -
    204 // we could validly allow futher entries into JSC from other threads, we only
    205 // need ensure that callbacks return in the reverse chronological order of the
    206 // order in which they were made - though implementing the less restrictive policy
    207 // would likely increase complexity and overhead.
    208 //
    209 
    210172// This function returns the number of locks that were dropped.
    211 unsigned JSLock::dropAllLocks()
     173unsigned JSLock::dropAllLocks(DropAllLocks* dropper)
    212174{
    213175    // Check if this thread is currently holding the lock.
    … …  
    216178        return 0;
    217179
    218     // Don't drop the locks if they've already been dropped once.
    219     // (If the prior drop came from another thread, and it resumed first,
    220     // it could trash our register file).
    221     if (m_lockDropDepth)
    222         return 0;
     180    ++m_lockDropDepth;
     181
     182    UNUSED_PARAM(dropper);
     183#if ENABLE(LLINT_C_LOOP)
     184    dropper->setDropDepth(m_lockDropDepth);
     185#endif
    223186
    224187    WTFThreadData& threadData = wtfThreadData();
    … …  
    227190    threadData.setSavedReservedZoneSize(m_vm->reservedZoneSize());
    228191
    229     // m_lockDropDepth is only incremented if any locks were dropped.
    230     ++m_lockDropDepth;
    231 
    232192    unsigned droppedLockCount = m_lockCount;
    233193    unlock(droppedLockCount);
    … …  
    236196}
    237197
    238 unsigned JSLock::dropAllLocksUnconditionally()
    239 {
    240     // Check if this thread is currently holding the lock.
    241     // FIXME: Maybe we want to require this, guard with an ASSERT?
    242     if (!currentThreadIsHoldingLock())
    243         return 0;
    244 
    245     WTFThreadData& threadData = wtfThreadData();
    246     threadData.setSavedStackPointerAtVMEntry(m_vm->stackPointerAtVMEntry);
    247     threadData.setSavedLastStackTop(m_vm->lastStackTop());
    248     threadData.setSavedReservedZoneSize(m_vm->reservedZoneSize());
    249 
    250     // m_lockDropDepth is only incremented if any locks were dropped.
    251     ++m_lockDropDepth;
    252 
    253     unsigned droppedLockCount = m_lockCount;
    254     unlock(droppedLockCount);
    255 
    256     return droppedLockCount;
    257 }
    258 
    259 void JSLock::grabAllLocks(unsigned droppedLockCount)
     198void JSLock::grabAllLocks(DropAllLocks* dropper, unsigned droppedLockCount)
    260199{
    261200    // If no locks were dropped, nothing to do!
    … …  
    265204    ASSERT(!currentThreadIsHoldingLock());
    266205    lock(droppedLockCount);
     206
     207    UNUSED_PARAM(dropper);
     208#if ENABLE(LLINT_C_LOOP)
     209    while (dropper->dropDepth() != m_lockDropDepth) {
     210        unlock(droppedLockCount);
     211        std::this_thread::yield();
     212        lock(droppedLockCount);
     213    }
     214#endif
    267215
    268216    --m_lockDropDepth;
    … …  
    274222}
    275223
    276 JSLock::DropAllLocks::DropAllLocks(ExecState* exec, AlwaysDropLocksTag alwaysDropLocks)
     224JSLock::DropAllLocks::DropAllLocks(ExecState* exec)
    277225    : m_droppedLockCount(0)
    278226    , m_vm(exec ? &exec->vm() : nullptr)
    … …  
    280228    if (!m_vm)
    281229        return;
    282 
    283     if (alwaysDropLocks)
    284         m_droppedLockCount = m_vm->apiLock().dropAllLocksUnconditionally();
    285     else
    286         m_droppedLockCount = m_vm->apiLock().dropAllLocks();
    287 }
    288 
    289 JSLock::DropAllLocks::DropAllLocks(VM* vm, AlwaysDropLocksTag alwaysDropLocks)
     230    m_droppedLockCount = m_vm->apiLock().dropAllLocks(this);
     231}
     232
     233JSLock::DropAllLocks::DropAllLocks(VM* vm)
    290234    : m_droppedLockCount(0)
    291235    , m_vm(vm)
    … …  
    293237    if (!m_vm)
    294238        return;
    295 
    296     if (alwaysDropLocks)
    297         m_droppedLockCount = m_vm->apiLock().dropAllLocksUnconditionally();
    298     else
    299         m_droppedLockCount = m_vm->apiLock().dropAllLocks();
     239    m_droppedLockCount = m_vm->apiLock().dropAllLocks(this);
    300240}
    301241
    … …  
    304244    if (!m_vm)
    305245        return;
    306     m_vm->apiLock().grabAllLocks(m_droppedLockCount);
     246    m_vm->apiLock().grabAllLocks(this, m_droppedLockCount);
    307247}
    308248

  • trunk/Source/JavaScriptCore/runtime/JSLock.h

    r163820 r163855  
    100100            WTF_MAKE_NONCOPYABLE(DropAllLocks);
    101101        public:
    102             // By default, we release all locks conditionally. Some clients, such as Mobile Safari,
    103             // may require that we release all locks unconditionally.
    104             enum AlwaysDropLocksTag { DontAlwaysDropLocks = 0, AlwaysDropLocks };
    105             JS_EXPORT_PRIVATE DropAllLocks(ExecState*, AlwaysDropLocksTag = DontAlwaysDropLocks);
    106             JS_EXPORT_PRIVATE DropAllLocks(VM*, AlwaysDropLocksTag = DontAlwaysDropLocks);
     102            JS_EXPORT_PRIVATE DropAllLocks(ExecState*);
     103            JS_EXPORT_PRIVATE DropAllLocks(VM*);
    107104            JS_EXPORT_PRIVATE ~DropAllLocks();
    108105           
     106#if ENABLE(LLINT_C_LOOP)
     107            void setDropDepth(unsigned depth) { m_dropDepth = depth; }
     108            unsigned dropDepth() const { return m_dropDepth; }
     109#endif
     110
    109111        private:
    110112            intptr_t m_droppedLockCount;
    111113            RefPtr<VM> m_vm;
     114#if ENABLE(LLINT_C_LOOP)
     115            unsigned m_dropDepth;
     116#endif
    112117        };
    113118
    … …  
    117122        void setOwnerThread(ThreadIdentifier owner) { m_ownerThread = owner; }
    118123
    119         unsigned dropAllLocks();
    120         unsigned dropAllLocksUnconditionally();
    121         void grabAllLocks(unsigned lockCount);
     124        unsigned dropAllLocks(DropAllLocks*);
     125        void grabAllLocks(DropAllLocks*, unsigned lockCount);
    122126
    123127        Mutex m_lock;

  • trunk/Source/WebCore/ChangeLog

    r163854 r163855  
     12014-02-10  Mark Lam  <mark.lam@apple.com>
     2
     3        Removing limitation on JSLock’s lockDropDepth.
     4        <https://webkit.org/b/128570>
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        No new tests.
     9
     10        * bindings/js/PageScriptDebugServer.cpp:
     11        (WebCore::PageScriptDebugServer::runEventLoopWhilePaused):
     12        * platform/ios/wak/WebCoreThread.mm:
     13        (SendDelegateMessage):
     14        (WebThreadRunOnMainThread):
     15        - No longer need to specify AlwaysDropLocks, because DropAllLocks is
     16          now always unconditional.
     17
    1182014-02-10  Benjamin Poulain  <benjamin@webkit.org>
    219

  • trunk/Source/WebCore/bindings/js/PageScriptDebugServer.cpp

    r163777 r163855  
    191191    {
    192192        if (WebThreadIsEnabled())
    193             JSC::JSLock::DropAllLocks dropAllLocks(WebCore::JSDOMWindowBase::commonVM(), JSC::JSLock::DropAllLocks::AlwaysDropLocks);
     193            JSC::JSLock::DropAllLocks dropAllLocks(WebCore::JSDOMWindowBase::commonVM());
    194194        WebRunLoopEnableNested();
    195195#endif

  • trunk/Source/WebCore/platform/ios/wak/WebCoreThread.mm

    r161603 r163855  
    211211        {
    212212            // Code block created to scope JSC::JSLock::DropAllLocks outside of WebThreadLock()
    213             JSC::JSLock::DropAllLocks dropAllLocks(WebCore::JSDOMWindowBase::commonVM(), JSC::JSLock::DropAllLocks::AlwaysDropLocks);
     213            JSC::JSLock::DropAllLocks dropAllLocks(WebCore::JSDOMWindowBase::commonVM());
    214214            _WebThreadUnlock();
    215215
    … …  
    249249    }
    250250
    251     JSC::JSLock::DropAllLocks dropAllLocks(WebCore::JSDOMWindowBase::commonVM(), JSC::JSLock::DropAllLocks::AlwaysDropLocks);
     251    JSC::JSLock::DropAllLocks dropAllLocks(WebCore::JSDOMWindowBase::commonVM());
    252252    _WebThreadUnlock();
    253253
Note: See TracChangeset for help on using the changeset viewer.