RFR: 8327743: JVM crash in hotspot/share/runtime/javaThread.cpp - failed: held monitor count should be equal to jni: 0 != 1 [v5]

[David Holmes]

On Wed, 10 Apr 2024 08:46:20 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Cleanup test leftovers
>
> src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp line 1:
> 
>> 1: /*
> 
> Copyright header needs update

I updated Oracle and SAP copyrights.

> src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp line 1673:
> 
>> 1671:     // Save return value potentially containing the exception oop
>> 1672:     Register ex_oop = R15_esp;   // nonvolatile register
>> 1673:     __ mr(ex_oop, R3_RET);
> 
> Please add `R15_esp` to the Kills section above in the header comment.

Done

> src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp line 1680:
> 
>> 1678:     // For vthreads we have to explicitly zero the JNI monitor count of the carrier
>> 1679:     // on termination. The held count is implicitly zeroed below when we restore from
>> 1680:     // the parent held count (which has to be zero).
> 
> This comment is not quite correct or a little imprecise, I found.
> 
>> the parent held count (which has to be zero)
> 
> I think technically the held count (_held_monitor_count) could be non-zero. Of course it would likely be bad if it was (holding a monitor while suspended is usually not good).
> 
> I thought it was like this:
> 
> 
> The JNI monitor count of the carrier thread is required to be zero when
> switching to the virtual thread. Here we are switching back to the carrier.
> We have to restore its JNI monitor count of zero.

In the general case the parent held count might be non-zero, but with vthreads and pinning it has to be zero. What we need to do with this code is ensure the held-count and jni-count are in sync: held-count >= jni-count. If the held-count could be non-zero here then we would not know what value to set in the jni-count to make sure the relationship is correct. It is only because we know the held-count has to be zero that we know we have to set the jni-count to zero.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18445#discussion_r1560298440
PR Review Comment: https://git.openjdk.org/jdk/pull/18445#discussion_r1560295882
PR Review Comment: https://git.openjdk.org/jdk/pull/18445#discussion_r1560293758