Should not access the thread states of none Java threads#346
Merged
zhengyu123 merged 11 commits intomainfrom Jan 29, 2026
Merged
Should not access the thread states of none Java threads#346zhengyu123 merged 11 commits intomainfrom
zhengyu123 merged 11 commits intomainfrom
Conversation
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jbachorik
reviewed
Jan 29, 2026
Comment on lines
+23
to
+26
| is_java_thread = raw_thread_state >= 4 && raw_thread_state < 12; | ||
|
|
||
| return is_java_thread ? convertJvmExecutionState(raw_thread_state) | ||
| : ExecutionMode::JVM; |
Collaborator
There was a problem hiding this comment.
If we are running this only for is_java_thread will we ever get to execute the 'false' branch here?
Contributor
Author
There was a problem hiding this comment.
Not quite sure what you mean - there is a second check is_java_thread = raw_thread_state >= 4 && raw_thread_state < 12; in old code that overwrites is_java_thread value, so it is possible to reach false branch.
Collaborator
There was a problem hiding this comment.
How can we get non-java specific states in a java thread?
Or is is_java_thread rather is_in_java_thread_state after the overwriting?
Contributor
Author
There was a problem hiding this comment.
The first is_java_thread check guarantees that this thread is a java - this block of code is inherited from old code.
If it fails the first is_java_thread check, then it is a JVM internal thread - always return ExecutionMode::JVM before we have better solution,
Collaborator
There was a problem hiding this comment.
Ok, I am fine. Just put a bit of explanatory doc there for the future. Otherwise, no objections to merging
jbachorik
approved these changes
Jan 29, 2026
Collaborator
jbachorik
left a comment
There was a problem hiding this comment.
Approved, pending the clarification about getThreadExecutionMode changes
jbachorik
reviewed
Jan 29, 2026
| switchThreadEvents(JVMTI_ENABLE); | ||
|
|
||
| // Initialize this thread | ||
| onThreadStart(nullptr, nullptr, nullptr); |
Collaborator
There was a problem hiding this comment.
Nit: May update the doc for this function to mention that it allows all-null values and what that means
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jbachorik
approved these changes
Jan 29, 2026
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.
What does this PR do?:
Profiler should not access the thread states of none Java threads.
Motivation:
Thread state is only defined for Java thread - accessing the thread state of a none Java thread can lead to crash, as the memory may not be valid.
Additional Notes:
Nativethreads, as they are largely running inNativestate.BaseContextWallClockTest.javais necessary, asJvmtiBasedContextWallClockTest.javais safepoint biased, threads are either inSafepointor inNativestatesHow to test the change?:
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!