RFR 8234613: JavaThread can escape back to Java from an ongoing handshake

[Patricio Chilano]

Thanks David!

Patricio
On 11/25/19 5:16 PM, David Holmes wrote:
> Hi Patricio,
>
> v2 looks good!
>
> Thanks,
> David
>
> On 26/11/2019 4:48 am, Patricio Chilano wrote:
>> Hi David,
>>
>> On 11/25/19 1:46 AM, David Holmes wrote:
>>> Hi Patricio,
>>>
>>> On 23/11/2019 4:25 am, Patricio Chilano wrote:
>>>> Hi,
>>>>
>>>> This patch aims to address a current bug where, given the right 
>>>> combination of handshakes and external suspend/resume, a JavaThread 
>>>> can transition from a safe state back to Java without blocking for 
>>>> a still-in-progress handshake. In the description of the bug I 
>>>> added an example, tracing the state changes of the JavaThread as it 
>>>> goes through the different transitions until it escapes the 
>>>> handshake. Currently, the window of time for this issue to happen 
>>>> is so small that we do not see actual failures running tests. 
>>>> Running test SuspendAtExit.java and adding some small delay before 
>>>> restoring the JavaThread state in 
>>>> java_suspend_self_with_safepoint_check() can demonstrate the issue.
>>>
>>> Good catch. This highlights how difficult it is to see where all the 
>>> thread-state-transitions are and reason about what can and can't 
>>> happen in a given sequence of code.
>>>
>>>> The proposed fix is to check again if we have a pending/in-progress 
>>>> handshake operation after executing ~ThreadInVMForHandshake().
>>>
>>> Minor nit but given we end up calling process_self_inner only after 
>>> it was determined the current _handshake has_operation(), then the 
>>> while loop should really be a do-while loop ?
>> Right, changed!
>>
>> Here is v2:
>> http://cr.openjdk.java.net/~pchilanomate/8234613/v02/webrev/ 
>> <http://cr.openjdk.java.net/~pchilanomate/8234613/v02/webrev/src/hotspot/share/runtime/handshake.cpp.udiff.html>
>>
>> Thanks for looking at this David!
>>
>> Patricio
>>> Thanks,
>>> David
>>> -----
>>>
>>>> Tested with mach5, tiers1-6 on all platforms (Linux, macOS, Windows 
>>>> and Solaris).
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8234613
>>>> Webrev: http://cr.openjdk.java.net/~pchilanomate/8234613/v01/webrev/
>>>>
>>>> Thanks,
>>>> Patricio
>>