RFR(s): 8210303: VM_HandshakeAllThreads fails assert with "failed: blocked and not walkable"

[Robbin Ehn]

Hi David,

On 10/3/18 1:49 PM, David Holmes wrote:
> On 3/10/2018 9:42 PM, Robbin Ehn wrote:
>> Hi all,
>>
>> Notice that I missed the extended suspend case:
>> http://cr.openjdk.java.net/~rehn/8210303/v2/full/
>> http://cr.openjdk.java.net/~rehn/8210303/v2/inc/
> 
> Won't a suspended thread be covered by the existing checks of the thread-state? 
> It should be blocked or in native.

I wish it was so:
Both JavaCallWrapper and ThreadInVMfromJava calls
JavaThread::handle_special_runtime_exit_condition with state _thread_in_Java.
We end in java_suspend_self which takes SR lock without safepoint check.

The thread is now in java but with ext suspend flag set.
This leads to very long stalls for the handshake and in some cases deadlock.
There might be others as well.

That case is handle with safepoints in 
ThreadSafepointState::examine_state_of_thread().

The normal check for this with handshakes is done in 
HandshakeState::vmthread_can_process_handshake() with the OR.

I was looking at adding the correct transitions in
handle_special_runtime_exit_condition. But I decided that I'll rather spend my
time implementing SR with handshakes instead, simplifying this.

Thanks, Robbin

> 
> David
> 
>> Re-running sanity with t1-5 on this.
>>
>> Thanks, Robbin
>>
>> On 10/3/18 1:10 PM, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> VM thread checks if it can processes a handshake for a JavaThread. That
>>> check will only return a stable value if the VM thread holds the handshake
>>> semaphore (or at safepoint). To avoid an unnecessary grabbing of the semaphore
>>> just to release it, the VM thread do an early check to see if there is any point
>>> to do the stable check. But the method SafepointSynchronize::safepoint_safe() is
>>> not suppose to handle unstable checks. This can causes a false positive from an
>>> assert in safepoint_safe().
>>>
>>> This change-set adds a local function for doing the unstable check without 
>>> asserts. I do not want to expose a generic method for doing unstable 
>>> safepoint safe test.
>>>
>>> Since asserts are not in release builds, there is no indication of a bug in JDK
>>> 11. But since 11 is a LTS, this should also be considered for back-porting.
>>>
>>> Note, in JDK 11 only ZGC uses handshakes, previously releases have no users 
>>> of handshakes.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~rehn/8210303/webrev/index.html
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8210303
>>>
>>> I could not reproduce it, sanity with t1-3 + handshake tests.
>>>
>>> Thanks, Robbin