Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions Source/WTF/wtf/posix/ThreadingPOSIX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include <wtf/StdLibExtras.h>
#include <wtf/ThreadingPrimitives.h>
#include <wtf/WTFConfig.h>
#include <wtf/threads/Signals.h>

#if OS(HAIKU)
#include <OS.h>
Expand Down Expand Up @@ -196,21 +197,30 @@ void Thread::signalHandlerSuspendResume(int, siginfo_t*, void* ucontext)
clearSuspendResumeRequest();

void* approximateStackPointer = currentStackPointer();
PlatformRegisters* registersOfOwnStack = nullptr;
if (!thread->m_stack.contains(approximateStackPointer)) {
// This happens if we use an alternative signal stack.
// 1. A user-defined signal handler is invoked with an alternative signal stack.
// 2. In the middle of the execution of the handler, we attempt to suspend the target thread.
// 3. A nested signal handler is executed.
// 4. The stack pointer saved in the machine context will be pointing to the alternative signal stack.
// In this case, we back off the suspension and retry a bit later.
thread->m_platformRegisters = nullptr;
globalSemaphoreForSuspendResume->post();
return;
#if USE(BUN_JSC_ADDITIONS) && HAVE(MACHINE_CONTEXT)
// The signal handler of WTF runs on the alternate signal stack (SA_ONSTACK), and it can wait
// there for a lock that the thread that suspends holds (SamplingProfiler::takeSample()): no
// retry would succeed. It publishes the registers that it interrupted.
registersOfOwnStack = registersInterruptedBySignalHandler();
#endif
if (!registersOfOwnStack) {
// This happens if we use an alternative signal stack.
// 1. A user-defined signal handler is invoked with an alternative signal stack.
// 2. In the middle of the execution of the handler, we attempt to suspend the target thread.
// 3. A nested signal handler is executed.
// 4. The stack pointer saved in the machine context will be pointing to the alternative signal stack.
// In this case, we back off the suspension and retry a bit later.
thread->m_platformRegisters = nullptr;
globalSemaphoreForSuspendResume->post();
return;
}
}

#if HAVE(MACHINE_CONTEXT)
ucontext_t* userContext = static_cast<ucontext_t*>(ucontext);
thread->m_platformRegisters = &registersFromUContext(userContext);
thread->m_platformRegisters = registersOfOwnStack ? registersOfOwnStack : &registersFromUContext(userContext);
#else
UNUSED_PARAM(ucontext);
PlatformRegisters platformRegisters { approximateStackPointer };
Expand Down
30 changes: 30 additions & 0 deletions Source/WTF/wtf/threads/Signals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include <wtf/DataLog.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/PlatformRegisters.h>
#include <wtf/Scope.h>
#include <wtf/ThreadGroup.h>
#include <wtf/Threading.h>
#include <wtf/WTFConfig.h>
Expand Down Expand Up @@ -481,6 +482,15 @@
g_wtfConfig.signalHandlers.add(signal, WTF::move(handler));
}

#if USE(BUN_JSC_ADDITIONS) && !OS(DARWIN)
static thread_local PlatformRegisters* interruptedRegisters;

PlatformRegisters* registersInterruptedBySignalHandler()
{
return interruptedRegisters;
}
#endif

static void jscSignalHandler(int sig, siginfo_t* info, void* ucontext)
{
Signal signal = fromSystemSignal(sig);
Expand Down Expand Up @@ -512,6 +522,19 @@
PlatformRegisters registers { };
#endif

#if USE(BUN_JSC_ADDITIONS) && !OS(DARWIN) && HAVE(MACHINE_CONTEXT)
// With SA_ONSTACK this handler runs on the alternate signal stack, where a suspension of this
// thread cannot read the state of the thread's own stack (Thread::signalHandlerSuspendResume()).
// These registers are that state, unless the interrupted code ran on the alternate stack too.
bool interruptedOwnStack = !(reinterpret_cast<ucontext_t*>(ucontext)->uc_stack.ss_flags & SS_ONSTACK);

Check failure on line 529 in Source/WTF/wtf/threads/Signals.cpp

View check run for this annotation

Claude / Claude Code Review

On Linux the fix in this push works or not per how the thread registered its sigaltstack, not per where the fault happened. Since kernel 4.7 uc_stack.ss_flags holds the flags passed to sigaltstack, so the test at Signals.cpp:529 is a per-thread constant.

On Linux the fix in this push works or not per how the thread registered its sigaltstack, not per where the fault happened. Since kernel 4.7 uc_stack.ss_flags holds the flags passed to sigaltstack, so the test at Signals.cpp:529 is a per-thread constant. Registered with SS_ONSTACK: registers are never published and the profiler hang remains. Registered with 0 or SS_AUTODISARM: a handler nested in another alternate-stack handler publishes registers whose sp is on that stack, which the base backed
if (interruptedOwnStack)
interruptedRegisters = &registers;
auto forgetInterruptedRegisters = makeScopeExit([&] {
if (interruptedOwnStack)
interruptedRegisters = nullptr;
});
#endif

bool didHandle = false;
bool restoreDefaultHandler = false;
handlers.forEachHandler(signal, [&] (const SignalHandler& handler) {
Expand Down Expand Up @@ -594,7 +617,14 @@
RELEASE_ASSERT(g_wtfConfig.isThreadSuspendResumeSignalConfigured);
result = sigdelset(&action.sa_mask, g_wtfConfig.sigThreadSuspendResume);
RELEASE_ASSERT(!result);
#if USE(BUN_JSC_ADDITIONS)
// On a thread that has an alternate signal stack, the handler runs on that stack. After a
// stack overflow the thread's own stack has no room for the signal frame: without this flag
// the kernel ends the process before this handler, or the one it chains to, can run.
action.sa_flags = SA_SIGINFO | SA_ONSTACK;
Comment thread
robobun marked this conversation as resolved.
Outdated
#else
action.sa_flags = SA_SIGINFO;
#endif
auto systemSignals = toSystemSignal(signal);
result = sigaction(std::get<0>(systemSignals), &action, &handlers.oldActions[offsetForSystemSignal(std::get<0>(systemSignals))]);
if (std::get<1>(systemSignals))
Expand Down
6 changes: 6 additions & 0 deletions Source/WTF/wtf/threads/Signals.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ class Thread;
void registerThreadForMachExceptionHandling(Thread&);
#endif // HAVE(MACH_EXCEPTIONS)

#if USE(BUN_JSC_ADDITIONS) && OS(UNIX) && !OS(DARWIN)
// While the signal handler of WTF runs on the alternate signal stack of the current thread: the
// registers of the code that it interrupted on the thread's own stack. Null at any other time.
WTF_EXPORT_PRIVATE PlatformRegisters* registersInterruptedBySignalHandler();
#endif

} // namespace WTF

#if HAVE(MACH_EXCEPTIONS)
Expand Down
Loading