8372039: post_sampled_object_alloc is called while lock is handled

[lmesnik]

The AOT allocates objects while holding lock. The jvmti events can't be posted in such case. The allocation sampling might be just temporary disabled while AOT objects are allocated.

I prefer to disable jvmti events for allocation only, not for AOT globally. If there are more events should be generated during AOT initialization, we might want to preserve them and post after initialization is completed.

The existing failure could be reproduced by running tests with jvmti stress agent and ZGC enabled. Like
make run-test JTREG_JVMTI_STRESS_AGENT=debugger=true TEST=gc/z/TestGarbageCollectorMXBean.java

Note:
I prelaced NoJvmtiVMObjectAllocMark, it was not used. Also it was incorrect. The
NoJvmtiEventsMark should be set even if jvmti events are not enable for this thread. Since jvmti events might be enabled just in the middle of the mark.

---

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-8372039: post_sampled_object_alloc is called while lock is handled (Bug - P2)

Reviewers

• Erik Österlund (@fisk - Reviewer) Review applies to f7e869b2
• Alex Menkov (@alexmenkov - Reviewer) Review applies to aaa0ccbb
• Serguei Spitsyn (@sspitsyn - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28544

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back lmesnik! 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]

@lmesnik 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:

8372039: post_sampled_object_alloc is called while lock is handled

Reviewed-by: sspitsyn, eosterlund, amenkov

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 82 new commits pushed to the master branch:

• b0a758f: 8372552: unhandled oop in the JvmtiEventController::set_user_enabled
• 8f0cb57: 8347831: Re-examine version check when cross linking
• 37cd8d6: 8371501: Change IAE to NPE in java.awt.image.Kernel when data is null
• ... and 79 more: https://git.openjdk.org/jdk/compare/847fbab7924848e0e88d112db1d5d0b71372d597...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]

@lmesnik 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

• 05: Full - Incremental (830b5f94)
• 04: Full - Incremental (aaa0ccbb)
• 03: Full - Incremental (f7e869b2)
• 02: Full - Incremental (1c070dc7)
• 01: Full - Incremental (67bcdf11)
• 00: Full (7662d282)

[fisk]

This is good, but we also use the _is_disable_suspend flag to disable JVMTI events for similar reasons, but only from the AOT thread. Now that we have a better more explicit boolean for disabling JVMTI events, perhaps we should stop toggling the _is_disable_suspend flag on the AOT thread, in favour of using this new more explicit mechanism.

Having said that, if we do, then the disabling mechanism needs to be able to cope with nesting. If you agree, then I don't mind if we want to do that refactoring separately from this fix though. The fix looks good otherwise.

[fisk]

Looks good.

[sspitsyn]

The fix looks good in general. I've requested a minor test update though.

[lmesnik]

@fisk, @alexmenkov, @sspitsyn Thank you for review and provided feedback
/integrate

[openjdk]

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

• b0a758f: 8372552: unhandled oop in the JvmtiEventController::set_user_enabled
• 8f0cb57: 8347831: Re-examine version check when cross linking
• 37cd8d6: 8371501: Change IAE to NPE in java.awt.image.Kernel when data is null
• ... and 79 more: https://git.openjdk.org/jdk/compare/847fbab7924848e0e88d112db1d5d0b71372d597...master

Your commit was automatically rebased without conflicts.

[openjdk]

@lmesnik Pushed as commit f5e4cd7.

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