8346120: VirtualThreadPinned event recorded for Object.wait may have wrong duration or may record second event

[AlanBateman]

A jdk.VirtualPinnedEvent JFR event is recorded by Object::wait when a virtual thread waits in Object.wait while pinned. The posting of the event in ObjectMonitor::wait is done after waiting but it can block again in enter/ReenterI before it re-acquires. If that happens then the event duration won't be accurate, and for the off-list case, the enter may be contended and a jdk.VirtualPinnedEvent event will be recorded for contended monitor enter. The changes to fix this are small: move the post of the event for Object.wait to end of the wait function, and only post an event for contended monitor enter when not waiting.

I've added a test to check the pinned event that is recored for several contended monitor enter and Object.wait cases. It checks the event duration and that only one event is recorded for each case.

---

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-8346120: VirtualThreadPinned event recorded for Object.wait may have wrong duration or may record second event (Bug - P4)

Reviewers

• Patricio Chilano Mateo (@pchilano - Reviewer) Review applies to 2dfee538
• David Holmes (@dholmes-ora - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22718

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

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

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

8346120: VirtualThreadPinned event recorded for Object.wait may have wrong duration or may record second event

Reviewed-by: dholmes, pchilanomate

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

• 87804f2: 8346294: Invalid lint category specified in compiler.properties
• 18d1d61: 8346046: Enable copyright header format check
• a7631cc: 8346235: RISC-V: Optimize bitwise AND with mask values
• 929d4a5: 8346231: RISC-V: Fix incorrect assertion in SharedRuntime::generate_handler_blob
• 3030230: 8346278: Clean up some flag handing in flags-cflags.m4
• fd0207d: 8345327: JDK 24 RDP1 L10n resource files update
• 31c3b19: 8346174: UMAX/UMIN are missing from XXXVector::reductionOperations
• c75b1d4: 8346082: Output JVMTI agent information in hserr files
• 51662c2: 8346232: Remove leftovers of the jar --normalize feature
• d335941: 8345678: compute_modifiers should not be in create_mirror
• ... and 8 more: https://git.openjdk.org/jdk/compare/dbffe33251da4472945d97ab54c7e3354d7f42d2...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]

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

• core-libs
• hotspot-runtime

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 (bb2217c1)
• 02: Full - Incremental (51ef032d)
• 01: Full - Incremental (47643726)
• 00: Full (2dfee538)

[pchilano]

Looks good to me.

[dholmes-ora]

So to be clear ... you want one pinned event, the duration of which spans the wait and the monitor reentry. If we are pinned for the wait then we must be pinned for the reentry for the same reason, so no need to report the same thing twice.

[AlanBateman]

So to be clear ... you want one pinned event, the duration of which spans the wait and the monitor reentry. If we are pinned for the wait then we must be pinned for the reentry for the same reason, so no need to report the same thing twice.

Right, and also it would be confusing to have a "contended monitor enter" event recorded with Object.wait as the top frame in the stack trace.

[dholmes-ora]

LGTM. Thanks

[AlanBateman]

/integrate

[openjdk]

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

• 466c00a: 8346234: javax/swing/text/DefaultEditorKit/4278839/bug4278839.java still fails in CI
• bd3c0be: 8268611: jar --validate should check targeted classes in MR-JAR files
• 87804f2: 8346294: Invalid lint category specified in compiler.properties
• 18d1d61: 8346046: Enable copyright header format check
• a7631cc: 8346235: RISC-V: Optimize bitwise AND with mask values
• 929d4a5: 8346231: RISC-V: Fix incorrect assertion in SharedRuntime::generate_handler_blob
• 3030230: 8346278: Clean up some flag handing in flags-cflags.m4
• fd0207d: 8345327: JDK 24 RDP1 L10n resource files update
• 31c3b19: 8346174: UMAX/UMIN are missing from XXXVector::reductionOperations
• c75b1d4: 8346082: Output JVMTI agent information in hserr files
• ... and 10 more: https://git.openjdk.org/jdk/compare/dbffe33251da4472945d97ab54c7e3354d7f42d2...master

Your commit was automatically rebased without conflicts.

[openjdk]

@AlanBateman Pushed as commit fbbc7c3.

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