Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly manage JNI resources in JVMTI wallclock sampler #168

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jbachorik
Copy link
Collaborator

@jbachorik jbachorik commented Jan 14, 2025

What does this PR do?:
It fixes the issue with the JVMTI wallclock sampler keep all threads in the system alive indefinitely

Motivation:
Resolve the corresponding memory leak. Make JVMTI wallclock sampler usable.

Additional Notes:
Due to stricter checks for thread state before collecting stacktrace and proper resource management, the #167 is not necessary any more.

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-10859

Unsure? Have a question? Request a review!

Copy link

github-actions bot commented Jan 14, 2025

🔧 Report generated by pr-comment-scanbuild

Scan-Build Report

User:runner@fv-az1361-385
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 18.1.3 (1ubuntu1)
Date:Fri Jan 24 15:05:01 2025

Bug Summary

Bug TypeQuantityDisplay?
All Bugs6
Logic error
Dereference of null pointer3
Garbage return value1
Suspicious operation
Bitwise shift1
Unused code
Dead nested assignment1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Suspicious operationBitwise shiftvmStructs.cppfind87216
Unused codeDead nested assignmentvmStructs.cppcheckNativeBinding9771
Logic errorDereference of null pointersafeAccess.hload3318
Logic errorDereference of null pointersymbols_linux.hElfParser12928
Logic errorDereference of null pointerflightRecorder.cppflush15138
Logic errorGarbage return valuewallClock.cpplocal16117

Copy link

github-actions bot commented Jan 14, 2025

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Warnings (7)

Style Violations (406)

@r1viollet
Copy link
Collaborator

Thanks for figuring things out! I'll try to run things to see how it behaves.
Is there a way we could generate a smoke test that would help identify this ? Or perhaps in a benchmarking way ?

@jbachorik jbachorik force-pushed the jb/wallclock_interval branch from 027351d to 61248da Compare January 24, 2025 11:24
@jbachorik jbachorik force-pushed the jb/wallclock_interval branch from 61248da to 522c37e Compare January 24, 2025 11:56
@jbachorik jbachorik marked this pull request as draft January 27, 2025 15:39
@jbachorik
Copy link
Collaborator Author

I have found some weird behaviour in JVMTI, especially when I try using RAII objects.
I need to investigate first - turning the PR to draft in the meantime.

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