RFR: 8256383: PlatformMutex::try_lock has different semantics on Windows and Posix

[David Holmes]

Hi Thomas,

Thanks for reviewing this.

On 17/11/2020 5:29 pm, Thomas Stuefe wrote:
> On Tue, 17 Nov 2020 05:41:44 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>> Mutex::try_lock has to allow for the possibility that the PlatformMutex::try_lock allows recursive locking.
>>
>> Added additional commentary on the semantics provided by Mutex and the PlatformMutex classes.
>>
>> Testing: GHA, mach5 tiers 1-3
> 
> Hi David,
> 
> LGTM. Minor remarks below.
> 
> Thanks, Thomas
> 
> src/hotspot/share/runtime/mutex.cpp line 147:
> 
>> 145:   // safepoint state, but can check blocking state.
>> 146:   check_block_state(self);
>> 147:   if (_owner == NULL && _lock.try_lock()) {
> 
> `_owner != self` may be a bit clearer, at the expense of maybe unnecessarily calling into platform code (but before, we did that unconditionally).

Right - see my response to Stefan.

> src/hotspot/share/runtime/mutex.cpp line 141:
> 
>> 139: // Returns true if thread succeeds in grabbing the lock, otherwise false.
>> 140: // Checking for a NULL owner avoids the call into platform code and hides the
>> 141: // potential difference in recursive locking behaviour on some platforms.
> 
> Maybe move comment down to 146?

I'll move it and modify it.

Thanks,
David
-----

> -------------
> 
> Marked as reviewed by stuefe (Reviewer).
> 
> PR: https://git.openjdk.java.net/jdk/pull/1247
>