RFR: 8324881: ObjectSynchronizer::inflate(Thread* current...) is invoked for non-current thread [v10]
Axel Boldt-Christmas
aboldtch at openjdk.org
Wed Feb 7 14:44:00 UTC 2024
On Tue, 6 Feb 2024 23:37:32 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:
>> Axel Boldt-Christmas 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 22 additional commits since the last revision:
>>
>> - Merge remote-tracking branch 'upstream_jdk/master' into JDK-8324881
>> - Update comment
>> - Fix comments
>> - Update test/jdk/com/sun/jdi/EATests.java
>>
>> Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>> - Update test/jdk/com/sun/jdi/EATests.java
>>
>> Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>> - Update test/jdk/com/sun/jdi/EATests.java
>>
>> Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>> - Update test/jdk/com/sun/jdi/EATests.java
>>
>> Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>> - Remove newline
>> - Update src/hotspot/share/runtime/synchronizer.hpp
>>
>> Co-authored-by: Richard Reingruber <richard.reingruber at sap.com>
>> - Improve EARelockingNestedInflated_03 for LM_LIGHTWEIGHT
>> - ... and 12 more: https://git.openjdk.org/jdk/compare/8d6ba3bf...b3cc401c
>
> src/hotspot/share/runtime/synchronizer.cpp line 525:
>
>> 523: return;
>> 524: }
>> 525: assert(monitor->is_being_async_deflated(), "must be");
>
> You've added this assertion here after the `enter_for` must have lost the
> race to async deflation, but you didn't add the same assertion below
> after L540 -> L542. Why not?
The assert was added because `ObjectMonitor::enter_for` was initially not constrained by only racing with deflation. Now however `ObjectMonitor::enter_for` does assert that it succeeds if it manage to lock out deflation. This assert should never be reached but I kept it as it shows intent.
I did not added it to the `ObjectSynchronizer::enter` at the time because it did not seem needed. But given that I kept it in 'ObjectSynchronizer::enter_for' to show intent, the same argument could be made for `ObjectSynchronizer::enter`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17626#discussion_r1481579128
More information about the hotspot-dev
mailing list