RFR: 8298733: Reconsider monitors_on_stack assert
Fredrik Bredberg
fbredberg at openjdk.org
Wed Apr 16 12:31:57 UTC 2025
On Tue, 15 Apr 2025 17:14:48 GMT, Patricio Chilano Mateo <pchilanomate 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.
>
> 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.
Not a leftover. I want `monitors_on_stack()` to be self contained, so if anyone else starts to use it they don't need to call `assert_frames_in_continuation_are_safe()` before calling `monitors_on_stack()`.
Also, the plan is to keep `assert_frames_in_continuation_are_safe()` in `unwind_frames()` when we remove legacy locking.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24655#discussion_r2046820772
More information about the hotspot-runtime-dev
mailing list