RFR: 8255678: Add Mutex::try_lock version without rank checks [v2]

[Patricio Chilano Mateo]

On Wed, 18 Nov 2020 01:27:47 GMT, David Holmes <dholmes at openjdk.org> wrote:

> Hi Patricio,
> This looks good to me, modulo the overlap with JDK-8256383 (the try_lock bug).
> One query below.
> Thanks,
> David
Hi David,

Thanks for looking at this.

> src/hotspot/share/runtime/mutex.cpp line 161:
> 
>> 159:   if (_owner == self) {
>> 160:     return false;
>> 161:   }
> 
> This is being fixed by https://github.com/openjdk/jdk/pull/1247

I'll wait until you push and then I'll merge. I still have to check the owner here though, because if the thread already owns the lock, calling check_rank() from try_lock will do the wrong check (it will assume it's being called from wait()). An alternative would be to add a boolean parameter to check_rank(), set only from try_lock() so I can differentiate the two calls.

> src/hotspot/share/runtime/mutex.cpp line 220:
> 
>> 218:   JavaThread* const self = JavaThread::current();
>> 219:   // Safepoint checking logically implies an active JavaThread.
>> 220:   guarantee(self->is_active_Java_thread(), "invariant");
> 
> Not clear why this is necessary here, and why it is a guarantee rather than assert ??

I looked at the history of this check. The guarantee has been since day1 on the hg repo as "self->is_Java_thread()" and then changed in 8226228 to "self->is_active_Java_thread()". I moved it at the beginning of wait() since it seems the right place to check it just after retrieving self. It doesn't make sense to keep it as a guarantee when every other check is an assert so I will change it.

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

PR: https://git.openjdk.java.net/jdk/pull/1242