Skip to content

Do not attempt to read JavaThread state from non-JavaThread threads#103

Merged
jbachorik merged 1 commit into
jb/asanfrom
jb/asan_wallclock
May 31, 2024
Merged

Do not attempt to read JavaThread state from non-JavaThread threads#103
jbachorik merged 1 commit into
jb/asanfrom
jb/asan_wallclock

Conversation

@jbachorik
Copy link
Copy Markdown
Collaborator

@jbachorik jbachorik commented May 31, 2024

What does this PR do?:
The wallclock signal handler is trying to read Java thread state from the current thread indiscriminately and this means we might be accessing invalid memory for non-Java threads.
Unfortunately, the only way to test whether we are in a Java or attached Java thread (which can be resolved to jthread and therefore reading its state is valid) is to try and get the JNI environment. And that's what we do here.

Motivation:
This issue is flagged by ASAN but it can also lead to very weird thread states reported for non-Java threads.

Additional Notes:
Asan CI run showing that this change is fixing the asan report - https://github.com/DataDog/java-profiler/actions/runs/9317890928

How to test the change?:

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: PROF-9838

Unsure? Have a question? Request a review!

@jbachorik jbachorik requested a review from richardstartin May 31, 2024 12:15
@github-actions
Copy link
Copy Markdown
Contributor

🔧 Report generated by pr-comment-scanbuild

Scan-Build Report

User:runner@fv-az1490-179
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 clean all
Clang Version:Ubuntu clang version 14.0.0-1ubuntu1.1
Date:Fri May 31 12:16:42 2024

Bug Summary

Bug TypeQuantityDisplay?
All Bugs5
Logic error
Assigned value is garbage or undefined1
Dereference of null pointer2
Unused code
Dead initialization1
Dead nested assignment1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Logic errorAssigned value is garbage or undefineddwarf.cppparseInstructions23520
Unused codeDead initializationlivenessTracker.cppcleanup_table451
Unused codeDead nested assignmentvmStructs.cppcheckNativeBinding8271
Logic errorDereference of null pointerflightRecorder.cppflush14058
Logic errorDereference of null pointersafeAccess.hload3518

@github-actions
Copy link
Copy Markdown
Contributor

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Warnings (5)

Style Violations (160)

@jbachorik jbachorik merged commit ac78bc0 into jb/asan May 31, 2024
@jbachorik jbachorik deleted the jb/asan_wallclock branch May 31, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants