From 70c605847496766b0ca59bee03ecadb74e54a159 Mon Sep 17 00:00:00 2001 From: "carlosgc@webkit.org" Date: Tue, 4 Apr 2017 16:12:17 +0000 Subject: [PATCH] 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): git-svn-id: http://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.16@214882 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/JavaScriptCore/heap/MachineStackMarker.cpp | 73 +++++++++++++--------- Source/JavaScriptCore/heap/MachineStackMarker.h | 41 +++++++++--- Source/JavaScriptCore/runtime/SamplingProfiler.cpp | 4 +- 4 files changed, 137 insertions(+), 41 deletions(-) diff --git a/Source/JavaScriptCore/heap/MachineStackMarker.cpp b/Source/JavaScriptCore/heap/MachineStackMarker.cpp index 65eb0cf0d9b..5fea745d2ad 100644 --- a/Source/JavaScriptCore/heap/MachineStackMarker.cpp +++ b/Source/JavaScriptCore/heap/MachineStackMarker.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #if OS(DARWIN) @@ -69,14 +70,14 @@ // We use SIGUSR2 to suspend and resume machine threads in JavaScriptCore. static const int SigThreadSuspendResume = SIGUSR2; static StaticLock globalSignalLock; -thread_local static std::atomic threadLocalCurrentThread; +thread_local static std::atomic threadLocalCurrentThread { nullptr }; static void pthreadSignalHandlerSuspendResume(int, siginfo_t*, void* ucontext) { // Touching thread local atomic types from signal handlers is allowed. - JSC::MachineThreads::Thread* thread = threadLocalCurrentThread.load(); + JSC::MachineThreads::ThreadData* threadData = threadLocalCurrentThread.load(); - if (thread->suspended.load(std::memory_order_acquire)) { + if (threadData->suspended.load(std::memory_order_acquire)) { // This is signal handler invocation that is intended to be used to resume sigsuspend. // So this handler invocation itself should not process. // @@ -88,9 +89,9 @@ static void pthreadSignalHandlerSuspendResume(int, siginfo_t*, void* ucontext) ucontext_t* userContext = static_cast(ucontext); #if CPU(PPC) - thread->suspendedMachineContext = *userContext->uc_mcontext.uc_regs; + threadData->suspendedMachineContext = *userContext->uc_mcontext.uc_regs; #else - thread->suspendedMachineContext = userContext->uc_mcontext; + threadData->suspendedMachineContext = userContext->uc_mcontext; #endif // Allow suspend caller to see that this thread is suspended. @@ -99,7 +100,7 @@ static void pthreadSignalHandlerSuspendResume(int, siginfo_t*, void* ucontext) // // And sem_post emits memory barrier that ensures that suspendedMachineContext is correctly saved. // http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_11 - sem_post(&thread->semaphoreForSuspendResume); + sem_post(&threadData->semaphoreForSuspendResume); // Reaching here, SigThreadSuspendResume is blocked in this handler (this is configured by sigaction's sa_mask). // So before calling sigsuspend, SigThreadSuspendResume to this thread is deferred. This ensures that the handler is not executed recursively. @@ -109,7 +110,7 @@ static void pthreadSignalHandlerSuspendResume(int, siginfo_t*, void* ucontext) sigsuspend(&blockedSignalSet); // Allow resume caller to see that this thread is resumed. - sem_post(&thread->semaphoreForSuspendResume); + sem_post(&threadData->semaphoreForSuspendResume); } #endif // USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN) @@ -215,18 +216,29 @@ MachineThreads::~MachineThreads() } } +static MachineThreads::ThreadData* threadData() +{ + static NeverDestroyed> threadData; + return threadData.get(); +} + +MachineThreads::Thread::Thread(ThreadData* threadData) + : data(threadData) +{ + ASSERT(threadData); +} + Thread* MachineThreads::Thread::createForCurrentThread() { - auto stackBounds = wtfThreadData().stack(); - return new Thread(getCurrentPlatformThread(), stackBounds.origin(), stackBounds.end()); + return new Thread(threadData()); } bool MachineThreads::Thread::operator==(const PlatformThread& other) const { #if OS(DARWIN) || OS(WINDOWS) - return platformThread == other; + return data->platformThread == other; #elif USE(PTHREADS) - return !!pthread_equal(platformThread, other); + return !!pthread_equal(data->platformThread, other); #else #error Need a way to compare threads on this platform #endif @@ -325,11 +337,13 @@ void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoot conservativeRoots.add(currentThreadState.stackTop, currentThreadState.stackOrigin, jitStubRoutines, codeBlocks); } -MachineThreads::Thread::Thread(const PlatformThread& platThread, void* base, void* end) - : platformThread(platThread) - , stackBase(base) - , stackEnd(end) +MachineThreads::ThreadData::ThreadData() { + auto stackBounds = wtfThreadData().stack(); + platformThread = getCurrentPlatformThread(); + stackBase = stackBounds.origin(); + stackEnd = stackBounds.end(); + #if OS(WINDOWS) ASSERT(platformThread == GetCurrentThreadId()); bool isSuccessful = @@ -362,7 +376,7 @@ MachineThreads::Thread::Thread(const PlatformThread& platThread, void* base, voi #endif } -MachineThreads::Thread::~Thread() +MachineThreads::ThreadData::~ThreadData() { #if OS(WINDOWS) CloseHandle(platformThreadHandle); @@ -371,7 +385,7 @@ MachineThreads::Thread::~Thread() #endif } -bool MachineThreads::Thread::suspend() +bool MachineThreads::ThreadData::suspend() { #if OS(DARWIN) kern_return_t result = thread_suspend(platformThread); @@ -408,7 +422,7 @@ bool MachineThreads::Thread::suspend() #endif } -void MachineThreads::Thread::resume() +void MachineThreads::ThreadData::resume() { #if OS(DARWIN) thread_resume(platformThread); @@ -439,9 +453,9 @@ void MachineThreads::Thread::resume() #endif } -size_t MachineThreads::Thread::getRegisters(Thread::Registers& registers) +size_t MachineThreads::ThreadData::getRegisters(ThreadData::Registers& registers) { - Thread::Registers::PlatformRegisters& regs = registers.regs; + ThreadData::Registers::PlatformRegisters& regs = registers.regs; #if OS(DARWIN) #if CPU(X86) unsigned user_count = sizeof(regs)/sizeof(int); @@ -496,7 +510,7 @@ size_t MachineThreads::Thread::getRegisters(Thread::Registers& registers) #endif } -void* MachineThreads::Thread::Registers::stackPointer() const +void* MachineThreads::ThreadData::Registers::stackPointer() const { #if OS(DARWIN) @@ -601,7 +615,7 @@ void* MachineThreads::Thread::Registers::stackPointer() const } #if ENABLE(SAMPLING_PROFILER) -void* MachineThreads::Thread::Registers::framePointer() const +void* MachineThreads::ThreadData::Registers::framePointer() const { #if OS(DARWIN) @@ -684,7 +698,7 @@ void* MachineThreads::Thread::Registers::framePointer() const #endif } -void* MachineThreads::Thread::Registers::instructionPointer() const +void* MachineThreads::ThreadData::Registers::instructionPointer() const { #if OS(DARWIN) @@ -765,7 +779,8 @@ void* MachineThreads::Thread::Registers::instructionPointer() const #error Need a way to get the instruction pointer for another thread on this platform #endif } -void* MachineThreads::Thread::Registers::llintPC() const + +void* MachineThreads::ThreadData::Registers::llintPC() const { // LLInt uses regT4 as PC. #if OS(DARWIN) @@ -858,9 +873,9 @@ void* MachineThreads::Thread::Registers::llintPC() const } #endif // ENABLE(SAMPLING_PROFILER) -void MachineThreads::Thread::freeRegisters(Thread::Registers& registers) +void MachineThreads::ThreadData::freeRegisters(ThreadData::Registers& registers) { - Thread::Registers::PlatformRegisters& regs = registers.regs; + ThreadData::Registers::PlatformRegisters& regs = registers.regs; #if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN) pthread_attr_destroy(®s.attribute); #else @@ -883,7 +898,7 @@ static inline int osRedZoneAdjustment() return redZoneAdjustment; } -std::pair MachineThreads::Thread::captureStack(void* stackTop) +std::pair MachineThreads::ThreadData::captureStack(void* stackTop) { char* begin = reinterpret_cast_ptr(stackBase); char* end = bitwise_cast(WTF::roundUpToMultipleOf(reinterpret_cast(stackTop))); @@ -971,12 +986,12 @@ bool MachineThreads::tryCopyOtherThreadStacks(LockHolder&, void* buffer, size_t } // Re-do the suspension to get the actual failure result for logging. - kern_return_t error = thread_suspend(thread->platformThread); + kern_return_t error = thread_suspend(thread->platformThread()); ASSERT(error != KERN_SUCCESS); WTFReportError(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, "JavaScript garbage collection encountered an invalid thread (err 0x%x): Thread [%d/%d: %p] platformThread %p.", - error, index, numberOfThreads, thread, reinterpret_cast(thread->platformThread)); + error, index, numberOfThreads, thread, reinterpret_cast(thread->platformThread())); // Put the invalid thread on the threadsToBeDeleted list. // We can't just delete it here because we have suspended other diff --git a/Source/JavaScriptCore/heap/MachineStackMarker.h b/Source/JavaScriptCore/heap/MachineStackMarker.h index a5a50870922..da979c582ec 100644 --- a/Source/JavaScriptCore/heap/MachineStackMarker.h +++ b/Source/JavaScriptCore/heap/MachineStackMarker.h @@ -74,14 +74,13 @@ public: JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads. - class Thread { + class ThreadData { WTF_MAKE_FAST_ALLOCATED; - Thread(const PlatformThread& platThread, void* base, void* end); - public: - ~Thread(); + ThreadData(); + ~ThreadData(); - static Thread* createForCurrentThread(); + static ThreadData* createForCurrentThread(); struct Registers { void* stackPointer() const; @@ -118,12 +117,9 @@ public: #else #error Need a thread register struct for this platform #endif - + PlatformRegisters regs; }; - - bool operator==(const PlatformThread& other) const; - bool operator!=(const PlatformThread& other) const { return !(*this == other); } bool suspend(); void resume(); @@ -131,7 +127,6 @@ public: void freeRegisters(Registers&); std::pair captureStack(void* stackTop); - Thread* next; PlatformThread platformThread; void* stackBase; void* stackEnd; @@ -145,6 +140,32 @@ public: #endif }; + class Thread { + WTF_MAKE_FAST_ALLOCATED; + Thread(ThreadData*); + + public: + using Registers = ThreadData::Registers; + + static Thread* createForCurrentThread(); + + bool operator==(const PlatformThread& other) const; + bool operator!=(const PlatformThread& other) const { return !(*this == other); } + + bool suspend() { return data->suspend(); } + void resume() { data->resume(); } + size_t getRegisters(Registers& regs) { return data->getRegisters(regs); } + void freeRegisters(Registers& regs) { data->freeRegisters(regs); } + std::pair captureStack(void* stackTop) { return data->captureStack(stackTop); } + + const PlatformThread& platformThread() { return data->platformThread; } + void* stackBase() const { return data->stackBase; } + void* stackEnd() const { return data->stackEnd; } + + Thread* next; + ThreadData* data; + }; + Lock& getLock() { return m_registeredThreadsMutex; } Thread* threadsListHead(const LockHolder&) const { ASSERT(m_registeredThreadsMutex.isLocked()); return m_registeredThreads; } Thread* machineThreadForCurrentThread(); diff --git a/Source/JavaScriptCore/runtime/SamplingProfiler.cpp b/Source/JavaScriptCore/runtime/SamplingProfiler.cpp index a8d953d6622..9326d7a0fc9 100644 --- a/Source/JavaScriptCore/runtime/SamplingProfiler.cpp +++ b/Source/JavaScriptCore/runtime/SamplingProfiler.cpp @@ -169,8 +169,8 @@ protected: { uint8_t* fpCast = bitwise_cast(exec); for (MachineThreads::Thread* thread = m_vm.heap.machineThreads().threadsListHead(m_machineThreadsLocker); thread; thread = thread->next) { - uint8_t* stackBase = static_cast(thread->stackBase); - uint8_t* stackLimit = static_cast(thread->stackEnd); + uint8_t* stackBase = static_cast(thread->stackBase()); + uint8_t* stackLimit = static_cast(thread->stackEnd()); RELEASE_ASSERT(stackBase); RELEASE_ASSERT(stackLimit); if (fpCast <= stackBase && fpCast >= stackLimit) -- 2.12.2