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