Changeset 214882 in webkit


Ignore:
Timestamp:
Apr 4, 2017 9:12:17 AM (7 years ago)
Author:
Carlos Garcia Campos
Message:

Merge r214319 - [JSC] MachineThreads does not consider situation that one thread has multiple VMs
https://bugs.webkit.org/show_bug.cgi?id=169819

Reviewed by Mark Lam.

The Linux port of PlatformThread suspend/resume mechanism relies on having a thread
specific singleton thread data, and was relying on MachineThreads::Thread to be this
thread specific singleton. But because MachineThreads::Thread is not a thread specific
singleton, we can get a deadlock in the GTK port's DatabaseProcess.

This patch fixes this issue by moving per thread data from MachineThreads::Thread to
MachineThreads::ThreadData, where there will only be one instance of
MachineThreads::ThreadData per thread. Each MachineThreads::Thread will now point to
the same MachineThreads::ThreadData for any given thread.

  • heap/MachineStackMarker.cpp:

(pthreadSignalHandlerSuspendResume):
(JSC::threadData):
(JSC::MachineThreads::Thread::Thread):
(JSC::MachineThreads::Thread::createForCurrentThread):
(JSC::MachineThreads::Thread::operator==):
(JSC::MachineThreads::ThreadData::ThreadData):
(JSC::MachineThreads::ThreadData::~ThreadData):
(JSC::MachineThreads::ThreadData::suspend):
(JSC::MachineThreads::ThreadData::resume):
(JSC::MachineThreads::ThreadData::getRegisters):
(JSC::MachineThreads::ThreadData::Registers::stackPointer):
(JSC::MachineThreads::ThreadData::Registers::framePointer):
(JSC::MachineThreads::ThreadData::Registers::instructionPointer):
(JSC::MachineThreads::ThreadData::Registers::llintPC):
(JSC::MachineThreads::ThreadData::freeRegisters):
(JSC::MachineThreads::ThreadData::captureStack):
(JSC::MachineThreads::tryCopyOtherThreadStacks):
(JSC::MachineThreads::Thread::~Thread): Deleted.
(JSC::MachineThreads::Thread::suspend): Deleted.
(JSC::MachineThreads::Thread::resume): Deleted.
(JSC::MachineThreads::Thread::getRegisters): Deleted.
(JSC::MachineThreads::Thread::Registers::stackPointer): Deleted.
(JSC::MachineThreads::Thread::Registers::framePointer): Deleted.
(JSC::MachineThreads::Thread::Registers::instructionPointer): Deleted.
(JSC::MachineThreads::Thread::Registers::llintPC): Deleted.
(JSC::MachineThreads::Thread::freeRegisters): Deleted.
(JSC::MachineThreads::Thread::captureStack): Deleted.

  • heap/MachineStackMarker.h:

(JSC::MachineThreads::Thread::operator!=):
(JSC::MachineThreads::Thread::suspend):
(JSC::MachineThreads::Thread::resume):
(JSC::MachineThreads::Thread::getRegisters):
(JSC::MachineThreads::Thread::freeRegisters):
(JSC::MachineThreads::Thread::captureStack):
(JSC::MachineThreads::Thread::platformThread):
(JSC::MachineThreads::Thread::stackBase):
(JSC::MachineThreads::Thread::stackEnd):

  • runtime/SamplingProfiler.cpp:

(JSC::FrameWalker::isValidFramePointer):

  • runtime/VMTraps.cpp:

(JSC::findActiveVMAndStackBounds):

Location:
releases/WebKitGTK/webkit-2.16/Source/JavaScriptCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • releases/WebKitGTK/webkit-2.16/Source/JavaScriptCore/ChangeLog

    r214871 r214882  
     12017-03-23  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [JSC] MachineThreads does not consider situation that one thread has multiple VMs
     4        https://bugs.webkit.org/show_bug.cgi?id=169819
     5
     6        Reviewed by Mark Lam.
     7
     8        The Linux port of PlatformThread suspend/resume mechanism relies on having a thread
     9        specific singleton thread data, and was relying on MachineThreads::Thread to be this
     10        thread specific singleton. But because MachineThreads::Thread is not a thread specific
     11        singleton, we can get a deadlock in the GTK port's DatabaseProcess.
     12
     13        This patch fixes this issue by moving per thread data from MachineThreads::Thread to
     14        MachineThreads::ThreadData, where there will only be one instance of
     15        MachineThreads::ThreadData per thread. Each MachineThreads::Thread will now point to
     16        the same MachineThreads::ThreadData for any given thread.
     17
     18        * heap/MachineStackMarker.cpp:
     19        (pthreadSignalHandlerSuspendResume):
     20        (JSC::threadData):
     21        (JSC::MachineThreads::Thread::Thread):
     22        (JSC::MachineThreads::Thread::createForCurrentThread):
     23        (JSC::MachineThreads::Thread::operator==):
     24        (JSC::MachineThreads::ThreadData::ThreadData):
     25        (JSC::MachineThreads::ThreadData::~ThreadData):
     26        (JSC::MachineThreads::ThreadData::suspend):
     27        (JSC::MachineThreads::ThreadData::resume):
     28        (JSC::MachineThreads::ThreadData::getRegisters):
     29        (JSC::MachineThreads::ThreadData::Registers::stackPointer):
     30        (JSC::MachineThreads::ThreadData::Registers::framePointer):
     31        (JSC::MachineThreads::ThreadData::Registers::instructionPointer):
     32        (JSC::MachineThreads::ThreadData::Registers::llintPC):
     33        (JSC::MachineThreads::ThreadData::freeRegisters):
     34        (JSC::MachineThreads::ThreadData::captureStack):
     35        (JSC::MachineThreads::tryCopyOtherThreadStacks):
     36        (JSC::MachineThreads::Thread::~Thread): Deleted.
     37        (JSC::MachineThreads::Thread::suspend): Deleted.
     38        (JSC::MachineThreads::Thread::resume): Deleted.
     39        (JSC::MachineThreads::Thread::getRegisters): Deleted.
     40        (JSC::MachineThreads::Thread::Registers::stackPointer): Deleted.
     41        (JSC::MachineThreads::Thread::Registers::framePointer): Deleted.
     42        (JSC::MachineThreads::Thread::Registers::instructionPointer): Deleted.
     43        (JSC::MachineThreads::Thread::Registers::llintPC): Deleted.
     44        (JSC::MachineThreads::Thread::freeRegisters): Deleted.
     45        (JSC::MachineThreads::Thread::captureStack): Deleted.
     46        * heap/MachineStackMarker.h:
     47        (JSC::MachineThreads::Thread::operator!=):
     48        (JSC::MachineThreads::Thread::suspend):
     49        (JSC::MachineThreads::Thread::resume):
     50        (JSC::MachineThreads::Thread::getRegisters):
     51        (JSC::MachineThreads::Thread::freeRegisters):
     52        (JSC::MachineThreads::Thread::captureStack):
     53        (JSC::MachineThreads::Thread::platformThread):
     54        (JSC::MachineThreads::Thread::stackBase):
     55        (JSC::MachineThreads::Thread::stackEnd):
     56        * runtime/SamplingProfiler.cpp:
     57        (JSC::FrameWalker::isValidFramePointer):
     58        * runtime/VMTraps.cpp:
     59        (JSC::findActiveVMAndStackBounds):
     60
    1612017-04-03  Mark Lam  <mark.lam@apple.com>
    262
  • releases/WebKitGTK/webkit-2.16/Source/JavaScriptCore/heap/MachineStackMarker.cpp

    r213037 r214882  
    3434#include <stdlib.h>
    3535#include <wtf/MainThread.h>
     36#include <wtf/NeverDestroyed.h>
    3637#include <wtf/StdLibExtras.h>
    3738
     
    7071static const int SigThreadSuspendResume = SIGUSR2;
    7172static StaticLock globalSignalLock;
    72 thread_local static std::atomic<JSC::MachineThreads::Thread*> threadLocalCurrentThread;
     73thread_local static std::atomic<JSC::MachineThreads::ThreadData*> threadLocalCurrentThread { nullptr };
    7374
    7475static void pthreadSignalHandlerSuspendResume(int, siginfo_t*, void* ucontext)
    7576{
    7677    // Touching thread local atomic types from signal handlers is allowed.
    77     JSC::MachineThreads::Thread* thread = threadLocalCurrentThread.load();
    78 
    79     if (thread->suspended.load(std::memory_order_acquire)) {
     78    JSC::MachineThreads::ThreadData* threadData = threadLocalCurrentThread.load();
     79
     80    if (threadData->suspended.load(std::memory_order_acquire)) {
    8081        // This is signal handler invocation that is intended to be used to resume sigsuspend.
    8182        // So this handler invocation itself should not process.
     
    8990    ucontext_t* userContext = static_cast<ucontext_t*>(ucontext);
    9091#if CPU(PPC)
    91     thread->suspendedMachineContext = *userContext->uc_mcontext.uc_regs;
    92 #else
    93     thread->suspendedMachineContext = userContext->uc_mcontext;
     92    threadData->suspendedMachineContext = *userContext->uc_mcontext.uc_regs;
     93#else
     94    threadData->suspendedMachineContext = userContext->uc_mcontext;
    9495#endif
    9596
     
    100101    // And sem_post emits memory barrier that ensures that suspendedMachineContext is correctly saved.
    101102    // http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_11
    102     sem_post(&thread->semaphoreForSuspendResume);
     103    sem_post(&threadData->semaphoreForSuspendResume);
    103104
    104105    // Reaching here, SigThreadSuspendResume is blocked in this handler (this is configured by sigaction's sa_mask).
     
    110111
    111112    // Allow resume caller to see that this thread is resumed.
    112     sem_post(&thread->semaphoreForSuspendResume);
     113    sem_post(&threadData->semaphoreForSuspendResume);
    113114}
    114115#endif // USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN)
     
    216217}
    217218
     219static MachineThreads::ThreadData* threadData()
     220{
     221    static NeverDestroyed<ThreadSpecific<MachineThreads::ThreadData, CanBeGCThread::True>> threadData;
     222    return threadData.get();
     223}
     224
     225MachineThreads::Thread::Thread(ThreadData* threadData)
     226    : data(threadData)
     227{
     228    ASSERT(threadData);
     229}
     230
    218231Thread* MachineThreads::Thread::createForCurrentThread()
    219232{
    220     auto stackBounds = wtfThreadData().stack();
    221     return new Thread(getCurrentPlatformThread(), stackBounds.origin(), stackBounds.end());
     233    return new Thread(threadData());
    222234}
    223235
     
    225237{
    226238#if OS(DARWIN) || OS(WINDOWS)
    227     return platformThread == other;
     239    return data->platformThread == other;
    228240#elif USE(PTHREADS)
    229     return !!pthread_equal(platformThread, other);
     241    return !!pthread_equal(data->platformThread, other);
    230242#else
    231243#error Need a way to compare threads on this platform
     
    326338}
    327339
    328 MachineThreads::Thread::Thread(const PlatformThread& platThread, void* base, void* end)
    329     : platformThread(platThread)
    330     , stackBase(base)
    331     , stackEnd(end)
    332 {
     340MachineThreads::ThreadData::ThreadData()
     341{
     342    auto stackBounds = wtfThreadData().stack();
     343    platformThread = getCurrentPlatformThread();
     344    stackBase = stackBounds.origin();
     345    stackEnd = stackBounds.end();
     346
    333347#if OS(WINDOWS)
    334348    ASSERT(platformThread == GetCurrentThreadId());
     
    363377}
    364378
    365 MachineThreads::Thread::~Thread()
     379MachineThreads::ThreadData::~ThreadData()
    366380{
    367381#if OS(WINDOWS)
     
    372386}
    373387
    374 bool MachineThreads::Thread::suspend()
     388bool MachineThreads::ThreadData::suspend()
    375389{
    376390#if OS(DARWIN)
     
    409423}
    410424
    411 void MachineThreads::Thread::resume()
     425void MachineThreads::ThreadData::resume()
    412426{
    413427#if OS(DARWIN)
     
    440454}
    441455
    442 size_t MachineThreads::Thread::getRegisters(Thread::Registers& registers)
    443 {
    444     Thread::Registers::PlatformRegisters& regs = registers.regs;
     456size_t MachineThreads::ThreadData::getRegisters(ThreadData::Registers& registers)
     457{
     458    ThreadData::Registers::PlatformRegisters& regs = registers.regs;
    445459#if OS(DARWIN)
    446460#if CPU(X86)
     
    497511}
    498512
    499 void* MachineThreads::Thread::Registers::stackPointer() const
     513void* MachineThreads::ThreadData::Registers::stackPointer() const
    500514{
    501515#if OS(DARWIN)
     
    602616
    603617#if ENABLE(SAMPLING_PROFILER)
    604 void* MachineThreads::Thread::Registers::framePointer() const
     618void* MachineThreads::ThreadData::Registers::framePointer() const
    605619{
    606620#if OS(DARWIN)
     
    685699}
    686700
    687 void* MachineThreads::Thread::Registers::instructionPointer() const
     701void* MachineThreads::ThreadData::Registers::instructionPointer() const
    688702{
    689703#if OS(DARWIN)
     
    766780#endif
    767781}
    768 void* MachineThreads::Thread::Registers::llintPC() const
     782
     783void* MachineThreads::ThreadData::Registers::llintPC() const
    769784{
    770785    // LLInt uses regT4 as PC.
     
    859874#endif // ENABLE(SAMPLING_PROFILER)
    860875
    861 void MachineThreads::Thread::freeRegisters(Thread::Registers& registers)
    862 {
    863     Thread::Registers::PlatformRegisters& regs = registers.regs;
     876void MachineThreads::ThreadData::freeRegisters(ThreadData::Registers& registers)
     877{
     878    ThreadData::Registers::PlatformRegisters& regs = registers.regs;
    864879#if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN)
    865880    pthread_attr_destroy(&regs.attribute);
     
    884899}
    885900
    886 std::pair<void*, size_t> MachineThreads::Thread::captureStack(void* stackTop)
     901std::pair<void*, size_t> MachineThreads::ThreadData::captureStack(void* stackTop)
    887902{
    888903    char* begin = reinterpret_cast_ptr<char*>(stackBase);
     
    972987               
    973988                // Re-do the suspension to get the actual failure result for logging.
    974                 kern_return_t error = thread_suspend(thread->platformThread);
     989                kern_return_t error = thread_suspend(thread->platformThread());
    975990                ASSERT(error != KERN_SUCCESS);
    976991
    977992                WTFReportError(__FILE__, __LINE__, WTF_PRETTY_FUNCTION,
    978993                    "JavaScript garbage collection encountered an invalid thread (err 0x%x): Thread [%d/%d: %p] platformThread %p.",
    979                     error, index, numberOfThreads, thread, reinterpret_cast<void*>(thread->platformThread));
     994                    error, index, numberOfThreads, thread, reinterpret_cast<void*>(thread->platformThread()));
    980995
    981996                // Put the invalid thread on the threadsToBeDeleted list.
  • releases/WebKitGTK/webkit-2.16/Source/JavaScriptCore/heap/MachineStackMarker.h

    r213037 r214882  
    7575    JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads.
    7676
    77     class Thread {
     77    class ThreadData {
    7878        WTF_MAKE_FAST_ALLOCATED;
    79         Thread(const PlatformThread& platThread, void* base, void* end);
    80 
    8179    public:
    82         ~Thread();
    83 
    84         static Thread* createForCurrentThread();
     80        ThreadData();
     81        ~ThreadData();
     82
     83        static ThreadData* createForCurrentThread();
    8584
    8685        struct Registers {
     
    119118#error Need a thread register struct for this platform
    120119#endif
    121            
     120
    122121            PlatformRegisters regs;
    123122        };
    124        
    125         bool operator==(const PlatformThread& other) const;
    126         bool operator!=(const PlatformThread& other) const { return !(*this == other); }
    127123
    128124        bool suspend();
     
    132128        std::pair<void*, size_t> captureStack(void* stackTop);
    133129
    134         Thread* next;
    135130        PlatformThread platformThread;
    136131        void* stackBase;
     
    146141    };
    147142
     143    class Thread {
     144        WTF_MAKE_FAST_ALLOCATED;
     145        Thread(ThreadData*);
     146
     147    public:
     148        using Registers = ThreadData::Registers;
     149
     150        static Thread* createForCurrentThread();
     151
     152        bool operator==(const PlatformThread& other) const;
     153        bool operator!=(const PlatformThread& other) const { return !(*this == other); }
     154
     155        bool suspend() { return data->suspend(); }
     156        void resume() { data->resume(); }
     157        size_t getRegisters(Registers& regs) { return data->getRegisters(regs); }
     158        void freeRegisters(Registers& regs) { data->freeRegisters(regs); }
     159        std::pair<void*, size_t> captureStack(void* stackTop) { return data->captureStack(stackTop); }
     160
     161        const PlatformThread& platformThread() { return data->platformThread; }
     162        void* stackBase() const { return data->stackBase; }
     163        void* stackEnd() const { return data->stackEnd; }
     164
     165        Thread* next;
     166        ThreadData* data;
     167    };
     168
    148169    Lock& getLock() { return m_registeredThreadsMutex; }
    149170    Thread* threadsListHead(const LockHolder&) const { ASSERT(m_registeredThreadsMutex.isLocked()); return m_registeredThreads; }
  • releases/WebKitGTK/webkit-2.16/Source/JavaScriptCore/runtime/SamplingProfiler.cpp

    r211546 r214882  
    170170        uint8_t* fpCast = bitwise_cast<uint8_t*>(exec);
    171171        for (MachineThreads::Thread* thread = m_vm.heap.machineThreads().threadsListHead(m_machineThreadsLocker); thread; thread = thread->next) {
    172             uint8_t* stackBase = static_cast<uint8_t*>(thread->stackBase);
    173             uint8_t* stackLimit = static_cast<uint8_t*>(thread->stackEnd);
     172            uint8_t* stackBase = static_cast<uint8_t*>(thread->stackBase());
     173            uint8_t* stackLimit = static_cast<uint8_t*>(thread->stackEnd());
    174174            RELEASE_ASSERT(stackBase);
    175175            RELEASE_ASSERT(stackLimit);
Note: See TracChangeset for help on using the changeset viewer.