On Sun, 15 Dec 2024 21:15:52 GMT, David Holmes <[email protected]> wrote:
>> Alan Bateman has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains seven additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into JDK-8346120
>> - Fix test comments
>> - testObjectWait2 test already owns lock
>> - Min duration check needs to have some tolerance
>> - Rename test, check more event fields
>> - Filter out VirtualThread.getAndClearInterrupt
>> - Initial commit
>
> src/hotspot/share/runtime/objectMonitor.cpp line 1864:
>
>> 1862:
>> 1863: if (ce != nullptr && ce->is_virtual_thread()) {
>> 1864: assert(result != freeze_ok, "sanity check");
>
> This assertion is so far from where `result` is set that I don't think it
> serves any purpose. We would have returned at line 1693 already. It was
> already a long way before you moved this.
It would be bug to call JavaThread::post_vthread_pinned_event with freeze_ok so
the asserts at both call sites were to catch that. Since this function has an
assert too then they are redundant at the usages so can be removed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22718#discussion_r1886303218
