diff --git a/ddprof-lib/src/main/cpp/ctimer_linux.cpp b/ddprof-lib/src/main/cpp/ctimer_linux.cpp index 6cf46b2a7d..515f4ee298 100644 --- a/ddprof-lib/src/main/cpp/ctimer_linux.cpp +++ b/ddprof-lib/src/main/cpp/ctimer_linux.cpp @@ -230,9 +230,7 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { } int tid = 0; - if (JVMThread::current() == nullptr - && current->inInitWindow()) { - current->tickInitWindow(); + if (tickInitWindowIfNeeded(current)) { errno = saved_errno; return; } @@ -240,17 +238,17 @@ void CTimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { current->noteCPUSample(Profiler::instance()->recordingEpoch()); tid = current->tid(); - Shims::instance().setSighandlerTid(tid); - - ExecutionEvent event; - event._execution_mode = getThreadExecutionMode(); - // Opted into JVMTI delegation; drop the sample if the JVM rejects the - // request (WRONG_PHASE if JFR is not recording, NOT_AVAILABLE if - // jdk.StackTraceRequest is disabled). recordSampleDelegated() bumps the - // failure counters; there is no fallback to ASGCT in this engine. - Profiler::instance()->recordSampleDelegated(ucontext, _interval, tid, - BCI_CPU, &event); - Shims::instance().setSighandlerTid(-1); + { + SighandlerTidScope sighandlerTid(tid); + ExecutionEvent event; + event._execution_mode = getThreadExecutionMode(); + // Opted into JVMTI delegation; drop the sample if the JVM rejects the + // request (WRONG_PHASE if JFR is not recording, NOT_AVAILABLE if + // jdk.StackTraceRequest is disabled). recordSampleDelegated() bumps the + // failure counters; there is no fallback to ASGCT in this engine. + Profiler::instance()->recordSampleDelegated(ucontext, _interval, tid, + BCI_CPU, &event); + } errno = saved_errno; } @@ -276,32 +274,30 @@ void CTimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { // Atomically try to enter critical section - prevents all reentrancy races CriticalSection cs(current); if (!cs.entered()) { + errno = saved_errno; return; // Another critical section is active, defer profiling } // we want to ensure memory order because of the possibility the instance gets // cleared if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) { + errno = saved_errno; return; } assert(!current->isDeepCrashHandler()); - // Guard against the race window between Profiler::registerThread() and - // thread_native_entry setting JVM TLS (PROF-13072): skip at most one signal - // per thread. Pure native threads (where JVMThread::current() is always null) - // are allowed through once the one-shot window expires. - if (JVMThread::current() == nullptr && current->inInitWindow()) { - current->tickInitWindow(); + if (tickInitWindowIfNeeded(current)) { errno = saved_errno; return; } current->noteCPUSample(Profiler::instance()->recordingEpoch()); int tid = current->tid(); - Shims::instance().setSighandlerTid(tid); - ExecutionEvent event; - event._execution_mode = getThreadExecutionMode(); - Profiler::instance()->recordSample(ucontext, _interval, tid, BCI_CPU, 0, - &event); - Shims::instance().setSighandlerTid(-1); + { + SighandlerTidScope sighandlerTid(tid); + ExecutionEvent event; + event._execution_mode = getThreadExecutionMode(); + Profiler::instance()->recordSample(ucontext, _interval, tid, BCI_CPU, 0, + &event); + } // we need to avoid spoiling the value of errno (tsan report) errno = saved_errno; } diff --git a/ddprof-lib/src/main/cpp/guards.h b/ddprof-lib/src/main/cpp/guards.h index f92de48c35..90f68351d1 100644 --- a/ddprof-lib/src/main/cpp/guards.h +++ b/ddprof-lib/src/main/cpp/guards.h @@ -23,6 +23,7 @@ #include #include "counters.h" +#include "debugSupport.h" class ProfiledThread; @@ -250,4 +251,30 @@ class SignalBlocker { SignalBlocker& operator=(const SignalBlocker&) = delete; }; +/** + * RAII guard around the span of a signal handler during which the current + * thread is the one being sampled. Sets Shims::instance().setSighandlerTid(tid) + * on construction and resets it to -1 on destruction, guaranteeing the reset + * happens on every return path out of the guarded scope. + * + * Must be scoped narrowly around exactly the existing set/reset span (right + * before recordSample and right after) rather than wrapped around the whole + * handler: widening the scope would change the window during which the + * sighandler tid is observably set for other consumers of Shims (e.g. + * crash-handler / re-entrant stack-walking code). + */ +class SighandlerTidScope { +public: + explicit SighandlerTidScope(int tid) { + Shims::instance().setSighandlerTid(tid); + } + ~SighandlerTidScope() { + Shims::instance().setSighandlerTid(-1); + } + + // Non-copyable + SighandlerTidScope(const SighandlerTidScope&) = delete; + SighandlerTidScope& operator=(const SighandlerTidScope&) = delete; +}; + #endif // _GUARDS_H diff --git a/ddprof-lib/src/main/cpp/itimer.cpp b/ddprof-lib/src/main/cpp/itimer.cpp index e4c02dcafd..decaeb2015 100644 --- a/ddprof-lib/src/main/cpp/itimer.cpp +++ b/ddprof-lib/src/main/cpp/itimer.cpp @@ -26,6 +26,7 @@ #include "threadLocalData.inline.h" #include "threadState.inline.h" #include "guards.h" +#include #include bool ITimer::_enabled = false; @@ -33,7 +34,8 @@ long ITimer::_interval; CStack ITimer::_cstack; void ITimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { - SIGNAL_HANDLER_GUARD_OR_DROP(); + int saved_errno = errno; + SIGNAL_HANDLER_GUARD_OR_DROP_WITH_ERRNO(saved_errno); // NOTE: ITimer uses setitimer(ITIMER_PROF) which delivers signals with // si_code==SI_KERNEL — no sival payload is available. The signal-origin // check implemented in CTimer/WallClock cannot be applied here. ITimer @@ -41,25 +43,31 @@ void ITimer::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { // feature addresses. Use CTimer (the default) when signal-origin // validation is required. InflightGuard inflight; - if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) + if (!__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) { + errno = saved_errno; return; + } ProfiledThread *current = SIGNAL_HANDLER_CURRENT_THREAD(); + assert(current != nullptr); // Atomically try to enter critical section - prevents all reentrancy races CriticalSection cs(current); if (!cs.entered()) { + errno = saved_errno; return; // Another critical section is active, defer profiling } current->noteCPUSample(Profiler::instance()->recordingEpoch()); int tid = current->tid(); - Shims::instance().setSighandlerTid(tid); - ExecutionEvent event; - event._execution_mode = getThreadExecutionMode(); - Profiler::instance()->recordSample(ucontext, _interval, tid, BCI_CPU, 0, - &event); - Shims::instance().setSighandlerTid(-1); + { + SighandlerTidScope sighandlerTid(tid); + ExecutionEvent event; + event._execution_mode = getThreadExecutionMode(); + Profiler::instance()->recordSample(ucontext, _interval, tid, BCI_CPU, 0, + &event); + } + errno = saved_errno; } Error ITimer::check(Arguments &args) { @@ -117,24 +125,23 @@ void ITimerJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { errno = saved_errno; return; } - if (JVMThread::current() == nullptr - && current->inInitWindow()) { - current->tickInitWindow(); + if (tickInitWindowIfNeeded(current)) { errno = saved_errno; return; } int tid = current->tid(); current->noteCPUSample(Profiler::instance()->recordingEpoch()); - Shims::instance().setSighandlerTid(tid); - - ExecutionEvent event; - event._execution_mode = getThreadExecutionMode(); - // setitimer(ITIMER_PROF) delivers SIGPROF to an arbitrary thread chosen by - // the OS, so ucontext may be from a JVM-internal thread. Pass nullptr to - // force the JVM into safepoint-based stack walking instead. - Profiler::instance()->recordSampleDelegated(nullptr, _interval, tid, - BCI_CPU, &event); - Shims::instance().setSighandlerTid(-1); + + { + SighandlerTidScope sighandlerTid(tid); + ExecutionEvent event; + event._execution_mode = getThreadExecutionMode(); + // setitimer(ITIMER_PROF) delivers SIGPROF to an arbitrary thread chosen by + // the OS, so ucontext may be from a JVM-internal thread. Pass nullptr to + // force the JVM into safepoint-based stack walking instead. + Profiler::instance()->recordSampleDelegated(nullptr, _interval, tid, + BCI_CPU, &event); + } errno = saved_errno; } diff --git a/ddprof-lib/src/main/cpp/jvmThread.h b/ddprof-lib/src/main/cpp/jvmThread.h index 2f5bd69104..9e13383aa8 100644 --- a/ddprof-lib/src/main/cpp/jvmThread.h +++ b/ddprof-lib/src/main/cpp/jvmThread.h @@ -10,6 +10,7 @@ #include #include "threadLocal.h" +#include "threadLocalData.h" /** * JVMThread represents a native JVM thread that is JVM implementation agnostic @@ -53,4 +54,20 @@ class JVMThread { static void* currentThreadSlow(); }; +// Shared init-window guard used by the CPU/wall profiling signal handlers. +// Guards against the race window between Profiler::registerThread() and +// thread_native_entry setting JVM TLS (PROF-13072): a pure native thread +// (where JVMThread::current() is always null) is allowed through once its +// one-shot init window has ticked down. Returns true if the caller should +// tick-and-return, in which case the tick has already happened; the caller +// remains responsible for restoring errno at its own return, since not all +// call sites save errno the same way. +static inline bool tickInitWindowIfNeeded(ProfiledThread* current) { + if (JVMThread::current() == nullptr && current->inInitWindow()) { + current->tickInitWindow(); + return true; + } + return false; +} + #endif // _JVMTHREAD_H diff --git a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp index 711a76f4c9..5cf1155206 100644 --- a/ddprof-lib/src/main/cpp/perfEvents_linux.cpp +++ b/ddprof-lib/src/main/cpp/perfEvents_linux.cpp @@ -764,9 +764,14 @@ class PerfFdRearmGuard { public: PerfFdRearmGuard(int fd, int tid) : _fd(fd), _tid(tid) {} ~PerfFdRearmGuard() { + // Constructed first among signalHandler's locals, so this destructs + // last -- after any errno restore the handler body performs. Save and + // restore errno here too, otherwise these calls silently clobber it. + int saved_errno = errno; PerfEvents::resetBuffer(_tid); ioctl(_fd, PERF_EVENT_IOC_RESET, 0); ioctl(_fd, PERF_EVENT_IOC_REFRESH, 1); + errno = saved_errno; } PerfFdRearmGuard(const PerfFdRearmGuard &) = delete; PerfFdRearmGuard &operator=(const PerfFdRearmGuard &) = delete; @@ -777,12 +782,13 @@ class PerfFdRearmGuard { }; void PerfEvents::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { + int saved_errno = errno; if (siginfo->si_code <= 0) { // Looks like an external signal; don't treat as a profiling event return; } PerfFdRearmGuard rearm(siginfo->si_fd, OS::threadId()); - SIGNAL_HANDLER_GUARD_OR_DROP(); + SIGNAL_HANDLER_GUARD_OR_DROP_WITH_ERRNO(saved_errno); InflightGuard inflight; // A thread with no ProfiledThread attached must never enter the critical @@ -797,20 +803,21 @@ void PerfEvents::signalHandler(int signo, siginfo_t *siginfo, void *ucontext) { // Atomically try to enter critical section - prevents all reentrancy races CriticalSection cs(current); if (!cs.entered()) { + errno = saved_errno; return; // Another critical section is active, defer profiling } current->noteCPUSample(Profiler::instance()->recordingEpoch()); int tid = current->tid(); if (__atomic_load_n(&_enabled, __ATOMIC_ACQUIRE)) { - Shims::instance().setSighandlerTid(tid); + SighandlerTidScope sighandlerTid(tid); u64 counter = readCounter(siginfo, ucontext); ExecutionEvent event; event._execution_mode = getThreadExecutionMode(); Profiler::instance()->recordSample(ucontext, counter, tid, BCI_CPU, 0, &event); - Shims::instance().setSighandlerTid(-1); } + errno = saved_errno; } Error PerfEvents::check(Arguments &args) { diff --git a/ddprof-lib/src/main/cpp/wallClock.cpp b/ddprof-lib/src/main/cpp/wallClock.cpp index 2afd90aa1c..dce9bd9e96 100644 --- a/ddprof-lib/src/main/cpp/wallClock.cpp +++ b/ddprof-lib/src/main/cpp/wallClock.cpp @@ -232,18 +232,16 @@ void WallClockASGCT::sharedSignalHandler(int signo, siginfo_t *siginfo, void WallClockASGCT::signalHandler(int signo, siginfo_t *siginfo, void *ucontext, u64 last_sample, ProfiledThread* current) { + int saved_errno = errno; assert(current != nullptr); // Atomically try to enter critical section - prevents all reentrancy races CriticalSection cs(current); if (!cs.entered()) { + errno = saved_errno; return; // Another critical section is active, defer profiling } - // Guard against the race window between Profiler::registerThread() and - // thread_native_entry setting JVM TLS (PROF-13072): skip at most one signal - // per thread. Pure native threads (where JVMThread::current() is always null) - // are allowed through once the one-shot window expires. - if (JVMThread::current() == nullptr && current->inInitWindow()) { - current->tickInitWindow(); + if (tickInitWindowIfNeeded(current)) { + errno = saved_errno; return; } // Once-per-run filter (wallprecheck=true): for untraced threads, exact @@ -254,51 +252,55 @@ void WallClockASGCT::signalHandler(int signo, siginfo_t *siginfo, void *ucontext // sampling instead of arming sampled_this_run. WallPrecheckResult precheck = prepareWallPrecheck(current, _precheck); if (precheck.suppress) { + errno = saved_errno; return; } int tid = current->tid(); - Shims::instance().setSighandlerTid(tid); - u64 call_trace_id = 0; - if (_collapsing) { - StackFrame frame(ucontext); - u64 spanId = 0, rootSpanId = 0; - // contextValid is not redundant with (spanId==0 && rootSpanId==0): a cleared - // context has spanId=0 and contextValid=true, while an uninitialized/mid-write - // thread has spanId=0 and contextValid=false. lookupWallclockCallTraceId uses - // contextValid to decide whether to update the sidecar _otel_local_root_span_id. - bool contextValid = ContextApi::get(spanId, rootSpanId); - call_trace_id = current->lookupWallclockCallTraceId( - (u64)frame.pc(), (u64)frame.sp(), - Profiler::instance()->recordingEpoch(), - contextValid, spanId, rootSpanId); - if (call_trace_id != 0) { - Counters::increment(SKIPPED_WALLCLOCK_UNWINDS); + + { + SighandlerTidScope sighandlerTid(tid); + u64 call_trace_id = 0; + if (_collapsing) { + StackFrame frame(ucontext); + u64 spanId = 0, rootSpanId = 0; + // contextValid is not redundant with (spanId==0 && rootSpanId==0): a cleared + // context has spanId=0 and contextValid=true, while an uninitialized/mid-write + // thread has spanId=0 and contextValid=false. lookupWallclockCallTraceId uses + // contextValid to decide whether to update the sidecar _otel_local_root_span_id. + bool contextValid = ContextApi::get(spanId, rootSpanId); + call_trace_id = current->lookupWallclockCallTraceId( + (u64)frame.pc(), (u64)frame.sp(), + Profiler::instance()->recordingEpoch(), + contextValid, spanId, rootSpanId); + if (call_trace_id != 0) { + Counters::increment(SKIPPED_WALLCLOCK_UNWINDS); + } } - } - ExecutionEvent event; - OSThreadState state = - precheck.observed_state_valid ? precheck.observed_state : getOSThreadState(); - ExecutionMode mode = getThreadExecutionMode(); - if (state == OSThreadState::UNKNOWN) { - if (inSyscall(ucontext)) { - state = OSThreadState::SYSCALL; - mode = ExecutionMode::SYSCALL; - } else { - state = OSThreadState::RUNNABLE; + ExecutionEvent event; + OSThreadState state = + precheck.observed_state_valid ? precheck.observed_state : getOSThreadState(); + ExecutionMode mode = getThreadExecutionMode(); + if (state == OSThreadState::UNKNOWN) { + if (inSyscall(ucontext)) { + state = OSThreadState::SYSCALL; + mode = ExecutionMode::SYSCALL; + } else { + state = OSThreadState::RUNNABLE; + } } + event._thread_state = state; + event._execution_mode = mode; + event._weight = precheck.unowned_weight; + u64 recorded_call_trace_id = 0; + bool recorded = Profiler::instance()->recordSample(ucontext, last_sample, tid, + BCI_WALL, call_trace_id, + &event, + &recorded_call_trace_id); + finishWallPrecheck(precheck, recorded, recorded_call_trace_id); + emitUnownedBlockedTailForWallPrecheck(tid, precheck); } - event._thread_state = state; - event._execution_mode = mode; - event._weight = precheck.unowned_weight; - u64 recorded_call_trace_id = 0; - bool recorded = Profiler::instance()->recordSample(ucontext, last_sample, tid, - BCI_WALL, call_trace_id, - &event, - &recorded_call_trace_id); - finishWallPrecheck(precheck, recorded, recorded_call_trace_id); - emitUnownedBlockedTailForWallPrecheck(tid, precheck); - Shims::instance().setSighandlerTid(-1); + errno = saved_errno; } Error BaseWallClock::start(Arguments &args) { @@ -441,16 +443,15 @@ void WallClockJvmti::sharedSignalHandler(int signo, siginfo_t *siginfo, void WallClockJvmti::signalHandler(int signo, siginfo_t *siginfo, void *ucontext, u64 last_sample, ProfiledThread* current) { + int saved_errno = errno; assert(current != nullptr); CriticalSection cs(current); if (!cs.entered()) { + errno = saved_errno; return; } - int saved_errno = errno; - if (JVMThread::current() == nullptr - && current->inInitWindow()) { - current->tickInitWindow(); + if (tickInitWindowIfNeeded(current)) { errno = saved_errno; return; } @@ -461,32 +462,33 @@ void WallClockJvmti::signalHandler(int signo, siginfo_t *siginfo, return; } int tid = current->tid(); - Shims::instance().setSighandlerTid(tid); - - ExecutionEvent event; - OSThreadState state = - precheck.observed_state_valid ? precheck.observed_state : getOSThreadState(); - ExecutionMode mode = getThreadExecutionMode(); - if (state == OSThreadState::UNKNOWN) { - if (inSyscall(ucontext)) { - state = OSThreadState::SYSCALL; - mode = ExecutionMode::SYSCALL; - } else { - state = OSThreadState::RUNNABLE; + + { + SighandlerTidScope sighandlerTid(tid); + ExecutionEvent event; + OSThreadState state = + precheck.observed_state_valid ? precheck.observed_state : getOSThreadState(); + ExecutionMode mode = getThreadExecutionMode(); + if (state == OSThreadState::UNKNOWN) { + if (inSyscall(ucontext)) { + state = OSThreadState::SYSCALL; + mode = ExecutionMode::SYSCALL; + } else { + state = OSThreadState::RUNNABLE; + } } + event._thread_state = state; + event._execution_mode = mode; + event._weight = precheck.unowned_weight; + // Pass nullptr ucontext so the JVM uses safepoint-based stack walking. + // Passing the signal-frame PC causes the extension to reject samples where + // the thread is currently inside JVM-internal (non-Java) code. + // JVMTI-delegated samples carry a correlation_id, not a call_trace_id, so + // unowned tail flushing remains limited to the ASGCT wall engine. + bool recorded = Profiler::instance()->recordSampleDelegated( + nullptr, last_sample, tid, BCI_WALL, &event); + finishWallPrecheck(precheck, recorded); } - event._thread_state = state; - event._execution_mode = mode; - event._weight = precheck.unowned_weight; - // Pass nullptr ucontext so the JVM uses safepoint-based stack walking. - // Passing the signal-frame PC causes the extension to reject samples where - // the thread is currently inside JVM-internal (non-Java) code. - // JVMTI-delegated samples carry a correlation_id, not a call_trace_id, so - // unowned tail flushing remains limited to the ASGCT wall engine. - bool recorded = Profiler::instance()->recordSampleDelegated( - nullptr, last_sample, tid, BCI_WALL, &event); - finishWallPrecheck(precheck, recorded); - Shims::instance().setSighandlerTid(-1); errno = saved_errno; }