RFR: 8298733: Reconsider monitors_on_stack assert

Patricio Chilano Mateo pchilanomate at openjdk.org
Tue Apr 15 17:19:43 UTC 2025


On Tue, 15 Apr 2025 09:38:05 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

> This PR aims to revert this [8298371: monitors_on_stack extracts unprocessed oops](https://github.com/openjdk/jdk/pull/11582) fix at the same time as we retain the ability to assert that we don't have any monitors on the stack when legacy locking is used.
> 
> It does so by moving an assert that ensures that we don't have any monitors on the stack to a place where all stack frames should have been processed. I.e. it's safe to check the frame for any monitor.
> 
> A new function called assert_frames_in_continuation_are_safe() asserts that the frames has been processed (i.e. it  safe to revert the [8298371](https://github.com/openjdk/jdk/pull/11582) fix). By keeping that assertion functionality separate from monitors_on_stack() we will be able to keep that assertion after we have removed legacy locking.
> 
> In order to recreate the original [problem](https://github.com/openjdk/jdk/pull/11582) and verifying that this PR works used, I used:
> `-XX:LockingMode=1 -XX:+UseZGC -XX:+ZVerifyOops -XX:ZCollectionIntervalMajor=0.001 Fuzz.java
> `
> 
> Also passes tier1-7.

Looks good to me, thanks.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 577:

> 575:   map.set_include_argument_oops(false);
> 576:   for (frame f = thread->last_frame(); Continuation::is_frame_in_continuation(ce, f); f = f.sender(&map)) {
> 577:     StackWatermark* watermark = StackWatermarkSet::get(thread, StackWatermarkKind::gc);

We could move this outside the loop.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 587:

> 585: #ifdef ASSERT
> 586: static bool monitors_on_stack(JavaThread* thread) {
> 587:   assert_frames_in_continuation_are_safe(thread);

Leftover? We just called this in the caller.

-------------

Marked as reviewed by pchilanomate (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24655#pullrequestreview-2769188713
PR Review Comment: https://git.openjdk.org/jdk/pull/24655#discussion_r2045099065
PR Review Comment: https://git.openjdk.org/jdk/pull/24655#discussion_r2045106994


More information about the hotspot-runtime-dev mailing list