8352773: JVMTI should disable events during java upcalls

[sspitsyn]

As noted in JDK-8352088, JVMTI GetThreadGroupChildren does an upcall to java. This results in aClassPrepare event the first time it does this, and these events can cause problems (deadlocks) for the debugger or debug agent. The JDK-8352088 was fixed to get rid of class loading during Java upcall from GetThreadGroupChildren. However, some other events can be generated as well. It is more safe to disable all JVMTI events during debugger-related upcalls originated by JVMTI.
The ClassPrepare events are important for the debug agent. So, an assert was added into ClassPrepare event generation to make sure there are no attempts to post this event during upcalls.
Some specific implementation details can be added to the first PR comment.

Testing:

• Verified with the test jdk/com/sun/jdi/EarlyThreadGroupChildrenTest.java that was added with the fix of JDK-8352088:
  • the assert described above is fired if the fix of JDK-8352088 is removed
  • the test is passed without if the fix of JDK-8352088 is removed and the assert is removed
• Ran mach5 tiers 1-6

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8352773: JVMTI should disable events during java upcalls (Bug - P3)

Reviewers

• Coleen Phillimore (@coleenp - Reviewer) Review applies to 82e99fae
• Leonid Mesnik (@lmesnik - Reviewer)
• David Holmes (@dholmes-ora - Reviewer)
• Chris Plummer (@plummercj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24539/head:pull/24539
$ git checkout pull/24539

Update a local copy of the PR:
$ git checkout pull/24539
$ git pull https://git.openjdk.org/jdk.git pull/24539/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24539

View PR using the GUI difftool:
$ git pr show -t 24539

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24539.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back sspitsyn! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@sspitsyn This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8352773: JVMTI should disable events during java upcalls

Reviewed-by: lmesnik, dholmes, cjplummer, coleenp

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 86 new commits pushed to the master branch:

• 4890b74: 8354216: Small cleanups relating to Log.DiagnosticHandler
• 7680f70: 8171508: Remove -Dsun.java.launcher.is_altjvm option
• 3131dd1: 8346465: Add a check in setData() to restrict the update of Built-In ICC_Profiles
• ... and 83 more: https://git.openjdk.org/jdk/compare/80ff7b9c9406c7845ecb3bc40910e92ccdd23ff2...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@sspitsyn The following labels will be automatically applied to this pull request:

• hotspot
• serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 03: Full - Incremental (908de6c0)
• 02: Full - Incremental (f0b70372)
• 01: Full - Incremental (342e8a78)
• 00: Full (82e99fae)

[AlanBateman]

If this goes ahead then it allows for a discussion about changing JVMTI InterruptThread to invoke Thread.interrupt when the target is a platform thread. As you know, there is a long standing issue here where threads blocked on interruptible channels not being awakened by JVMTI InterruptThread.

[sspitsyn]

If this goes ahead then it allows for a discussion about changing JVMTI InterruptThread to invoke Thread.interrupt when the target is a platform thread. As you know, there is a long standing issue here where threads blocked on interruptible channels not being awakened by JVMTI InterruptThread.

Yes, this fix is already including an update for interrupts. I'll add a comment with the fix details tomorrow.

[coleenp]

So this is another case where you have to ignore JVMTI event like in VTMS transitions? It looks like a good way to fix this in general.

[lmesnik]

Looks good.

[sspitsyn]

Coleen and Leonid, thank you for review!

[sspitsyn]

So this is another case where you have to ignore JVMTI event like in VTMS transitions? It looks like a good way to fix this in general.

Yes. This is a long standing issue which is good to fix now.

[dholmes-ora]

This looks reasonable to me. Thanks.

[sspitsyn]

David, thank you for review!

[sspitsyn]

I've pushed a minor update to fix a build time error for minimal VM.

[sspitsyn]

Chris, thank you for review! Thank you for re-approvals guys!

[sspitsyn]

/integrate

[openjdk]

Going to push as commit 1c34f3c.
Since your change was applied there have been 90 commits pushed to the master branch:

• 7bb8b17: 8347472: Correct Attribute traversal and writing for Code attributes
• 83c7d3b: 8351740: Clean up some code around initialization of encoding properties
• e35f67b: 8285888: Clarify that java.net.http.HttpClient do NOT support Digest authentication
• ... and 87 more: https://git.openjdk.org/jdk/compare/80ff7b9c9406c7845ecb3bc40910e92ccdd23ff2...master

Your commit was automatically rebased without conflicts.

[openjdk]

@sspitsyn Pushed as commit 1c34f3c.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.