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

[Daniel D. Daugherty]

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/

src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp
     No comments.

src/hotspot/share/prims/jni.cpp
     L4139:   // This thread will not be safepoint check, since it has
         nit: s/not be/not do a/

src/hotspot/share/runtime/handshake.cpp
     No comments.

src/hotspot/share/runtime/mutex.cpp
     L190:   // Safepoint checking logically implies active java_thread.
         nit: s/java_thread/JavaThread/

src/hotspot/share/runtime/mutexLocker.cpp
     No comments.

src/hotspot/share/runtime/thread.cpp
     No comments.

src/hotspot/share/runtime/thread.hpp
     No comments.

src/hotspot/share/runtime/threadSMR.cpp
     No comments.

src/hotspot/share/runtime/vmOperations.cpp
     No comments.

src/hotspot/share/utilities/vmError.cpp
     You may want this comment:

     L1794:     Threads_lock->try_lock();
                // The VM is going to die so no need to unlock Thread_lock.

Thumbs up. I only have comment changes/additions so no need for
a new webrev if you choose to make those changes.

Dan



>
> 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
>