RFR: 8226228: Make Threads_lock an always safepoint checked lock.

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Wed Aug 7 21:46:07 UTC 2019

Robbin,  This looks good.  Thank you for fixing Threads_lock to not be a 
sometimes lock.  That helps us.


I also had a comment about the try_lock.  But it doesn't seem like it 
would change behaviour.  If you get the lock, it is never unlocked 
though.  Could it deadlock error handling through another path?  It's 
probably okay.

- MutexLocker ml(Threads_lock->owned_by_self() ? NULL : Threads_lock, 
+ if (!Threads_lock->owned_by_self()) {
+ Threads_lock->try_lock();
+ }


In one of my patches, I moved this code so I can move it again. Maybe 
the NoSafepointVerifier checks should check is_active_java_thread() now.

    // Clear unhandled oops in JavaThreads so we get a crash right away.
- if (self->is_Java_thread()) {
+ if (self->is_active_Java_thread()) {

Ship it!

On 8/6/19 2:29 AM, Robbin Ehn wrote:
> Hi guys,
> Let's pick this up :) Been a while...
> I think that we were pretty happy after some explaining !?
> Here is full, let's call it v2:
> http://cr.openjdk.java.net/~rehn/8226228/v2/webrev/
> Passes t1-5.
> Thanks, Robbin
> On 2019-06-17 13:21, Robbin Ehn wrote:
>> Hi all, please review.
>> A safepoint checked lock always check for safepoint if the locker is an
>> JavaThread. If it's not a JavaThread, the thread are not 
>> participating in the
>> safepoint protocol thus safepoints are not checked.
>> A JavaThread that is not on _the_ ThreadsList is not participating in 
>> the safepoint protocol and for most purposes should be treat as 
>> NonJavaThread.
>> This applies to whether we should check for a safepoint.
>> This changeset suggest adding a method for checking this.
>> We can use that method in the mutex code to determine if we should 
>> treat locker
>> as JavaThread or a NonJavaThread.
>> The only problematic scenario is when we are an active JavaThread in 
>> vmError.cpp but don't want to safepoint, but want the ThreadsList to 
>> be stable.
>> I choose a best effort approach.
>> Alternatives could be stop freeing the backing array if there is an 
>> an error or
>> letting the thread running the error code become an inactive 
>> JavaThread thus
>> avoiding safepointing.
>> This fix also helps converting Heap_lock to an always lock (Heap_lock 
>> needed
>> changes not included).
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8226228
>> Code:
>> http://cr.openjdk.java.net/~rehn/8226228/webrev/index.html
>> Passes t1-3.
>> Thanks, Robbin

More information about the hotspot-runtime-dev mailing list