check thread is a Java thread before accessing the thread state#4
Merged
richardstartin merged 1 commit intoAug 3, 2023
mainDataDog/java-profiler:mainfrom
rgs/non-java-thread-execution-modeDataDog/java-profiler:rgs/non-java-thread-execution-modeCopy head branch name to clipboard
Merged
check thread is a Java thread before accessing the thread state#4richardstartin merged 1 commit intomainDataDog/java-profiler:mainfrom rgs/non-java-thread-execution-modeDataDog/java-profiler:rgs/non-java-thread-execution-modeCopy head branch name to clipboard
richardstartin merged 1 commit into
mainDataDog/java-profiler:mainfrom
rgs/non-java-thread-execution-modeDataDog/java-profiler:rgs/non-java-thread-execution-modeCopy head branch name to clipboard
Conversation
Contributor
3231176 to
2339d3c
Compare
Contributor
|
🔧 Report generated by pr-comment-scanbuild Scan-Build Report
Bug Summary
Reports
|
2339d3c to
0503745
Compare
0503745 to
910db67
Compare
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
Contributor
|
🔧 Report generated by pr-comment-stresstests
ResultsCapturingLambdas.capturingLambda [command='cpu=1us,wall=1us,memory=1048576:a']🔍
DumpRecording.dumpRecording [command='cpu=1us,wall=1us', nodeCount=1024]🔍
GraphMutation.mutateGraph [command='cpu=1us,wall=1us', nodeCount=1024]🔍
NanoTime.nanoTime [command='cpu=1us,wall=1us']🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=10]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=100]🔍
TracedParallelWork.work [command='cpu=1us,wall=1us,attributes=tag0;tag1', tagCardinality=1000]🔍
|
jbachorik
approved these changes
Aug 3, 2023
jbachorik
added a commit
that referenced
this pull request
May 25, 2026
The always-on _in_signal_handler_depth TLS variable, accessed first from inside our SIGPROF/SIGVTALRM handlers via SIGNAL_HANDLER_GUARD(), was declared with the default global-dynamic TLS model. On first access in a given thread glibc lazily allocates the dtv slot via malloc() and takes the heap lock — both async-signal-unsafe. Reproduced deterministically on Graal aarch64 (glibc 17-graal debug) running ClassGCTest: SIGPROF arrived on the VM Thread while Graal's JVMCI compiler held the malloc heap lock through c2v_notifyCompilerPhaseEvent. Stack: #2 __libc_malloc -- waiting on heap lock #4 allocate_dtv_entry #7 _dl_tlsdesc_dynamic #8 TLS wrapper for _in_signal_handler_depth #9 SignalHandlerScope::SignalHandlerScope #10 CTimer::signalHandler The heap holder is itself blocked at a safepoint waiting for VM Thread to check in, and VM Thread is stuck in malloc -> full process deadlock. Switch the variable to the initial-exec TLS model so the loader allocates its slot from the static TLS surplus at libjavaProfiler.so load time. Every existing thread is fixed up at dlopen and every new thread receives the slot at pthread_create. Access is then a register-relative load — async-signal-safe, lock-free, malloc-free. Also narrow the type to uint8_t (realistic max depth ~3) to make the intent explicit; alignment-wise this is the same slot. Refresher tick reduced from 5 s to 500 ms so a library lazily loaded from signal context becomes resolvable by the stack walker within half a second. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jbachorik
added a commit
that referenced
this pull request
May 25, 2026
Code review surfaced 7 findings — 4 CONFIRMED, 3 PLAUSIBLE. This commit addresses all 7: #8 (CONFIRMED) — split isInSignalContext into a strict variant. dlopen_hook was treating null ProfiledThread as "in signal" and deferring refresh on every dlopen from uninstrumented JVM threads (VM Thread, JIT, GC), delaying wasmtime sigaction patching by up to 500 ms. Add isInTrackedSignalContext() that returns false on null (only true when one of our SignalHandlerScopes is positively on the stack); dlopen_hook now uses it so JVM-internal threads get synchronous refresh again. isInSignalContext() retains its conservative semantics for any future caller that wants AS-safe-by-default. #7 (CONFIRMED) — switchLibraryTrap was called before startRefresher despite the invariant comment. Reorder so the refresher is running before the trap can fire. #9 (CONFIRMED) — DEBUG_ASSERT_NOT_IN_SIGNAL was on the 1-arg and 2-arg Dictionary::lookup overloads but missed the 4-arg form that actually mallocs. bounded_lookup's runtime-decided for_insert path was uncovered. Move the assertion into the 4-arg lookup, gated on for_insert (read-only lookups are AS-safe). #4 (CONFIRMED) — Comments referenced "REFRESH_INTERVAL_NS (5s)" but the actual constant is 500 ms. Fix both stale mentions. #13 (PLAUSIBLE) — SIGNAL_HANDLER_GUARD_RELEASE before chaining leaves depth == 0 inside a chained handler that returns normally; DEBUG_ASSERT_NOT_IN_SIGNAL inside such a handler would not fire. Document the trade-off in segvHandler — the longjmp safety property is more important than the sanitizer coverage gap, which is bounded to third-party signal handler code we don't own. #2 (PLAUSIBLE) — refresherLoop used OS::sleep without an EINTR loop; any unmasked signal (SIGCHLD, SIGURG, RT signals) would cause premature ticks. Wrap the sleep in an explicit elapsed-time loop using OS::nanotime so the refresher ticks at 500 ms regardless of stray signals. #14 (PLAUSIBLE) — refresherLoop published _refresher_tid before blocking SIGPROF/SIGVTALRM; a stale per-thread timer from a previous lifecycle could fire on the refresher during the window. Block signals first, then publish the TID. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The thread state is a
JavaThreadproperty in VMStructs, use the existence ofJNIEnvto filter out JVM threads.