Changeset 213238 in webkit


Ignore:
Timestamp:
Mar 1, 2017 12:15:08 PM (7 years ago)
Author:
mark.lam@apple.com
Message:

[Re-landing] Change JSLock to stash PlatformThread instead of std::thread::id.
https://bugs.webkit.org/show_bug.cgi?id=168996

Reviewed by Filip Pizlo and Saam Barati.

PlatformThread is more useful because it allows us to:

  1. find the MachineThreads::Thread which is associated with it.
  2. suspend / resume threads.
  3. send a signal to a thread.

We can't do those with std::thread::id. We will need one or more of these
capabilities to implement non-polling VM traps later.

Update: Since we don't have a canonical "uninitialized" value for PlatformThread,
we now have a JSLock::m_hasOwnerThread flag that is set to true if and only the
m_ownerThread value is valid. JSLock::currentThreadIsHoldingLock() now checks
JSLock::m_hasOwnerThread before doing the thread identity comparison.

  • JavaScriptCore.xcodeproj/project.pbxproj:
  • heap/MachineStackMarker.cpp:

(JSC::MachineThreads::Thread::createForCurrentThread):
(JSC::MachineThreads::machineThreadForCurrentThread):
(JSC::MachineThreads::removeThread):
(JSC::MachineThreads::Thread::suspend):
(JSC::MachineThreads::tryCopyOtherThreadStacks):
(JSC::getCurrentPlatformThread): Deleted.

  • heap/MachineStackMarker.h:
  • runtime/JSCellInlines.h:

(JSC::JSCell::classInfo):

  • runtime/JSLock.cpp:

(JSC::JSLock::JSLock):
(JSC::JSLock::lock):
(JSC::JSLock::unlock):
(JSC::JSLock::currentThreadIsHoldingLock): Deleted.

  • runtime/JSLock.h:

(JSC::JSLock::ownerThread):
(JSC::JSLock::currentThreadIsHoldingLock):

  • runtime/PlatformThread.h: Added.

(JSC::currentPlatformThread):

  • runtime/VM.cpp:

(JSC::VM::~VM):

  • runtime/VM.h:

(JSC::VM::ownerThread):

  • runtime/Watchdog.cpp:

(JSC::Watchdog::setTimeLimit):
(JSC::Watchdog::shouldTerminate):
(JSC::Watchdog::startTimer):
(JSC::Watchdog::stopTimer):

  • tools/JSDollarVMPrototype.cpp:

(JSC::JSDollarVMPrototype::currentThreadOwnsJSLock):

  • tools/VMInspector.cpp:
Location:
trunk/Source/JavaScriptCore
Files:
1 added
12 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r213233 r213238  
     12017-03-01  Mark Lam  <mark.lam@apple.com>
     2
     3        [Re-landing] Change JSLock to stash PlatformThread instead of std::thread::id.
     4        https://bugs.webkit.org/show_bug.cgi?id=168996
     5
     6        Reviewed by Filip Pizlo and Saam Barati.
     7
     8        PlatformThread is more useful because it allows us to:
     9        1. find the MachineThreads::Thread which is associated with it.
     10        2. suspend / resume threads.
     11        3. send a signal to a thread.
     12
     13        We can't do those with std::thread::id.  We will need one or more of these
     14        capabilities to implement non-polling VM traps later.
     15
     16        Update: Since we don't have a canonical "uninitialized" value for PlatformThread,
     17        we now have a JSLock::m_hasOwnerThread flag that is set to true if and only the
     18        m_ownerThread value is valid.  JSLock::currentThreadIsHoldingLock() now checks
     19        JSLock::m_hasOwnerThread before doing the thread identity comparison.
     20
     21        * JavaScriptCore.xcodeproj/project.pbxproj:
     22        * heap/MachineStackMarker.cpp:
     23        (JSC::MachineThreads::Thread::createForCurrentThread):
     24        (JSC::MachineThreads::machineThreadForCurrentThread):
     25        (JSC::MachineThreads::removeThread):
     26        (JSC::MachineThreads::Thread::suspend):
     27        (JSC::MachineThreads::tryCopyOtherThreadStacks):
     28        (JSC::getCurrentPlatformThread): Deleted.
     29        * heap/MachineStackMarker.h:
     30        * runtime/JSCellInlines.h:
     31        (JSC::JSCell::classInfo):
     32        * runtime/JSLock.cpp:
     33        (JSC::JSLock::JSLock):
     34        (JSC::JSLock::lock):
     35        (JSC::JSLock::unlock):
     36        (JSC::JSLock::currentThreadIsHoldingLock): Deleted.
     37        * runtime/JSLock.h:
     38        (JSC::JSLock::ownerThread):
     39        (JSC::JSLock::currentThreadIsHoldingLock):
     40        * runtime/PlatformThread.h: Added.
     41        (JSC::currentPlatformThread):
     42        * runtime/VM.cpp:
     43        (JSC::VM::~VM):
     44        * runtime/VM.h:
     45        (JSC::VM::ownerThread):
     46        * runtime/Watchdog.cpp:
     47        (JSC::Watchdog::setTimeLimit):
     48        (JSC::Watchdog::shouldTerminate):
     49        (JSC::Watchdog::startTimer):
     50        (JSC::Watchdog::stopTimer):
     51        * tools/JSDollarVMPrototype.cpp:
     52        (JSC::JSDollarVMPrototype::currentThreadOwnsJSLock):
     53        * tools/VMInspector.cpp:
     54
    1552017-03-01  Saam Barati  <sbarati@apple.com>
    256
  • trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

    r213231 r213238  
    24242424                FED94F2E171E3E2300BE77A4 /* Watchdog.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FED94F2B171E3E2300BE77A4 /* Watchdog.cpp */; };
    24252425                FED94F2F171E3E2300BE77A4 /* Watchdog.h in Headers */ = {isa = PBXBuildFile; fileRef = FED94F2C171E3E2300BE77A4 /* Watchdog.h */; settings = {ATTRIBUTES = (Private, ); }; };
     2426                FEE43FCE1E6641710077D6D1 /* PlatformThread.h in Headers */ = {isa = PBXBuildFile; fileRef = FEE43FCD1E6641400077D6D1 /* PlatformThread.h */; settings = {ATTRIBUTES = (Private, ); }; };
    24262427                FEF040511AAE662D00BD28B0 /* CompareAndSwapTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FEF040501AAE662D00BD28B0 /* CompareAndSwapTest.cpp */; };
    24272428                FEFD6FC61D5E7992008F2F0B /* JSStringInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = FEFD6FC51D5E7970008F2F0B /* JSStringInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
     
    50045005                FEDA50D41B97F442009A3B4F /* PingPongStackOverflowTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = PingPongStackOverflowTest.cpp; path = API/tests/PingPongStackOverflowTest.cpp; sourceTree = "<group>"; };
    50055006                FEDA50D51B97F4D9009A3B4F /* PingPongStackOverflowTest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = PingPongStackOverflowTest.h; path = API/tests/PingPongStackOverflowTest.h; sourceTree = "<group>"; };
     5007                FEE43FCD1E6641400077D6D1 /* PlatformThread.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PlatformThread.h; sourceTree = "<group>"; };
    50065008                FEF040501AAE662D00BD28B0 /* CompareAndSwapTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = CompareAndSwapTest.cpp; path = API/tests/CompareAndSwapTest.cpp; sourceTree = "<group>"; };
    50075009                FEF040521AAEC4ED00BD28B0 /* CompareAndSwapTest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = CompareAndSwapTest.h; path = API/tests/CompareAndSwapTest.h; sourceTree = "<group>"; };
     
    67336735                                0FE228EA1436AB2300196C48 /* Options.cpp */,
    67346736                                0FE228EB1436AB2300196C48 /* Options.h */,
     6737                                FEE43FCD1E6641400077D6D1 /* PlatformThread.h */,
    67356738                                868916A9155F285400CB2B9A /* PrivateName.h */,
    67366739                                147341DF1DC2CE9600AA29BA /* ProgramExecutable.cpp */,
     
    91759178                                705B41AC1A6E501E00716757 /* Symbol.h in Headers */,
    91769179                                705B41AE1A6E501E00716757 /* SymbolConstructor.h in Headers */,
     9180                                FEE43FCE1E6641710077D6D1 /* PlatformThread.h in Headers */,
    91779181                                996B73271BDA08EF00331B84 /* SymbolConstructor.lut.h in Headers */,
    91789182                                705B41B01A6E501E00716757 /* SymbolObject.h in Headers */,
  • trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp

    r213231 r213238  
    180180}
    181181   
    182 static inline PlatformThread getCurrentPlatformThread()
    183 {
    184 #if OS(DARWIN)
    185     return pthread_mach_thread_np(pthread_self());
    186 #elif OS(WINDOWS)
    187     return GetCurrentThreadId();
    188 #elif USE(PTHREADS)
    189     return pthread_self();
    190 #endif
    191 }
    192 
    193182MachineThreads::MachineThreads()
    194183    : m_registeredThreads(0)
     
    215204{
    216205    auto stackBounds = wtfThreadData().stack();
    217     return new Thread(getCurrentPlatformThread(), stackBounds.origin(), stackBounds.end());
     206    return new Thread(currentPlatformThread(), stackBounds.origin(), stackBounds.end());
    218207}
    219208
     
    251240{
    252241    LockHolder lock(m_registeredThreadsMutex);
    253     PlatformThread platformThread = getCurrentPlatformThread();
     242    PlatformThread platformThread = currentPlatformThread();
    254243    for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
    255244        if (*thread == platformThread)
     
    283272#endif
    284273
    285         machineThreads->removeThreadIfFound(getCurrentPlatformThread());
     274        machineThreads->removeThreadIfFound(currentPlatformThread());
    286275    }
    287276}
     
    376365    return threadIsSuspended;
    377366#elif USE(PTHREADS)
    378     ASSERT_WITH_MESSAGE(getCurrentPlatformThread() != platformThread, "Currently we don't support suspend the current thread itself.");
     367    ASSERT_WITH_MESSAGE(currentPlatformThread() != platformThread, "Currently we don't support suspend the current thread itself.");
    379368    {
    380369        // During suspend, suspend or resume should not be executed from the other threads.
     
    949938    *size = 0;
    950939
    951     PlatformThread currentPlatformThread = getCurrentPlatformThread();
     940    PlatformThread platformThread = currentPlatformThread();
    952941    int numberOfThreads = 0; // Using 0 to denote that we haven't counted the number of threads yet.
    953942    int index = 1;
     
    956945    Thread* previousThread = nullptr;
    957946    for (Thread* thread = m_registeredThreads; thread; index++) {
    958         if (*thread != currentPlatformThread) {
     947        if (*thread != platformThread) {
    959948            bool success = thread->suspend();
    960949#if OS(DARWIN)
     
    1000989
    1001990    for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
    1002         if (*thread != currentPlatformThread)
     991        if (*thread != platformThread)
    1003992            tryCopyOtherThreadStack(thread, buffer, capacity, size);
    1004993    }
    1005994
    1006995    for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
    1007         if (*thread != currentPlatformThread)
     996        if (*thread != platformThread)
    1008997            thread->resume();
    1009998    }
  • trunk/Source/JavaScriptCore/heap/MachineStackMarker.h

    r213231 r213238  
    2222#pragma once
    2323
     24#include "PlatformThread.h"
    2425#include "RegisterState.h"
    2526#include <wtf/Lock.h>
     
    2728#include <wtf/ScopedLambda.h>
    2829#include <wtf/ThreadSpecific.h>
    29 
    30 #if OS(DARWIN)
    31 #include <mach/thread_act.h>
    32 #endif
    3330
    3431#if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN)
     
    4340#endif
    4441#endif
    45 
    46 #if OS(DARWIN)
    47 typedef mach_port_t PlatformThread;
    48 #elif OS(WINDOWS)
    49 typedef DWORD PlatformThread;
    50 #elif USE(PTHREADS)
    51 typedef pthread_t PlatformThread;
    52 #endif // OS(DARWIN)
    5342
    5443namespace JSC {
  • trunk/Source/JavaScriptCore/runtime/JSCellInlines.h

    r213231 r213238  
    279279    // invalid). If mutatorState() == MutatorState::Running, then we're not currently sweeping, and therefore cannot be
    280280    // destructing the object. The GC thread or JIT threads, unlike the mutator thread, are able to access classInfo
    281     // independent of whether the mutator thread is sweeping or not. Hence, we also check for ownerThread() !=
    282     // std::this_thread::get_id() to allow the GC thread or JIT threads to pass this assertion.
    283     ASSERT(vm.heap.mutatorState() != MutatorState::Sweeping || vm.apiLock().ownerThread() != std::this_thread::get_id());
     281    // independent of whether the mutator thread is sweeping or not. Hence, we also check for !currentThreadIsHoldingAPILock()
     282    // to allow the GC thread or JIT threads to pass this assertion.
     283    ASSERT(vm.heap.mutatorState() != MutatorState::Sweeping || !vm.currentThreadIsHoldingAPILock());
    284284    return structure(vm)->classInfo();
    285285}
  • trunk/Source/JavaScriptCore/runtime/JSLock.cpp

    r213231 r213238  
    7676
    7777JSLock::JSLock(VM* vm)
    78     : m_ownerThreadID(std::thread::id())
    79     , m_lockCount(0)
     78    : m_lockCount(0)
    8079    , m_lockDropDepth(0)
    8180    , m_vm(vm)
     
    111110    }
    112111
    113     m_ownerThreadID = std::this_thread::get_id();
     112    m_ownerThread = currentPlatformThread();
     113    WTF::storeStoreFence();
     114    m_hasOwnerThread = true;
    114115    ASSERT(!m_lockCount);
    115116    m_lockCount = lockCount;
     
    169170
    170171    if (!m_lockCount) {
    171         m_ownerThreadID = std::thread::id();
     172        m_hasOwnerThread = false;
    172173        m_lock.unlock();
    173174    }
     
    201202{
    202203    exec->vm().apiLock().unlock();
    203 }
    204 
    205 bool JSLock::currentThreadIsHoldingLock()
    206 {
    207     return m_ownerThreadID == std::this_thread::get_id();
    208204}
    209205
  • trunk/Source/JavaScriptCore/runtime/JSLock.h

    r213231 r213238  
    2121#pragma once
    2222
     23#include "PlatformThread.h"
    2324#include <mutex>
    24 #include <thread>
    2525#include <wtf/Assertions.h>
    2626#include <wtf/Lock.h>
     
    9494    VM* vm() { return m_vm; }
    9595
    96     std::thread::id ownerThread() const { return m_ownerThreadID; }
    97     JS_EXPORT_PRIVATE bool currentThreadIsHoldingLock();
     96    std::optional<PlatformThread> ownerThread() const
     97    {
     98        if (m_hasOwnerThread)
     99            return m_ownerThread;
     100        return { };
     101    }
     102    bool currentThreadIsHoldingLock() { return m_hasOwnerThread && m_ownerThread == currentPlatformThread(); }
    98103
    99104    void willDestroyVM(VM*);
     
    127132
    128133    Lock m_lock;
    129     std::thread::id m_ownerThreadID;
     134    // We cannot make m_ownerThread an optional (instead of pairing it with an explicit
     135    // m_hasOwnerThread) because currentThreadIsHoldingLock() may be called from a
     136    // different thread, and an optional is vulnerable to races.
     137    // See https://bugs.webkit.org/show_bug.cgi?id=169042#c6
     138    bool m_hasOwnerThread { false };
     139    PlatformThread m_ownerThread;
    130140    intptr_t m_lockCount;
    131141    unsigned m_lockDropDepth;
  • trunk/Source/JavaScriptCore/runtime/VM.cpp

    r213231 r213238  
    393393    m_perBytecodeProfiler = nullptr;
    394394
    395     ASSERT(m_apiLock->currentThreadIsHoldingLock());
     395    ASSERT(currentThreadIsHoldingAPILock());
    396396    m_apiLock->willDestroyVM(this);
    397397    heap.lastChanceToFinalize();
  • trunk/Source/JavaScriptCore/runtime/VM.h

    r213231 r213238  
    612612#endif
    613613
    614     std::thread::id ownerThread() const { return m_apiLock->ownerThread(); }
    615 
    616614    JS_EXPORT_PRIVATE void resetDateCache();
    617615
     
    725723    bool isSafeToRecurseSoftCLoop() const;
    726724#endif // !ENABLE(JIT)
     725
     726    std::optional<PlatformThread> ownerThread() const { return m_apiLock->ownerThread(); }
    727727
    728728    JS_EXPORT_PRIVATE void throwException(ExecState*, Exception*);
  • trunk/Source/JavaScriptCore/runtime/Watchdog.cpp

    r213231 r213238  
    5656    ShouldTerminateCallback callback, void* data1, void* data2)
    5757{
    58     ASSERT(m_vm->ownerThread() == std::this_thread::get_id());
     58    ASSERT(m_vm->currentThreadIsHoldingAPILock());
    5959
    6060    m_timeLimit = limit;
     
    6969bool Watchdog::shouldTerminate(ExecState* exec)
    7070{
    71     ASSERT(m_vm->ownerThread() == std::this_thread::get_id());
     71    ASSERT(m_vm->currentThreadIsHoldingAPILock());
    7272    // FIXME: Will unindent the following before landing. Leaving indented for now to minimize the code diff.
    7373    {
     
    139139{
    140140    ASSERT(m_hasEnteredVM);
    141     ASSERT(m_vm->ownerThread() == std::this_thread::get_id());
     141    ASSERT(m_vm->currentThreadIsHoldingAPILock());
    142142    ASSERT(hasTimeLimit());
    143143    ASSERT(timeLimit <= m_timeLimit);
     
    169169{
    170170    ASSERT(m_hasEnteredVM);
    171     ASSERT(m_vm->ownerThread() == std::this_thread::get_id());
     171    ASSERT(m_vm->currentThreadIsHoldingAPILock());
    172172    m_cpuDeadline = noTimeLimit;
    173173}
  • trunk/Source/JavaScriptCore/tools/JSDollarVMPrototype.cpp

    r213231 r213238  
    4545bool JSDollarVMPrototype::currentThreadOwnsJSLock(ExecState* exec)
    4646{
    47     return exec->vm().apiLock().currentThreadIsHoldingLock();
     47    return exec->vm().currentThreadIsHoldingAPILock();
    4848}
    4949
  • trunk/Source/JavaScriptCore/tools/VMInspector.cpp

    r213231 r213238  
    140140    bool hasTimeout = false;
    141141    iterate([&] (VM& vm) {
    142         if (!vm.apiLock().currentThreadIsHoldingLock())
     142        if (!vm.currentThreadIsHoldingAPILock())
    143143            return FunctorStatus::Continue;
    144144
Note: See TracChangeset for help on using the changeset viewer.