RFR: 8217618: JVM TI SuspendThread doesn't suspend the current thread before returning

[David Holmes]

On 27/01/2019 6:33 am, dean.long at oracle.com wrote:
> I think there is going to be a race with resume wherever you put the 
> self-suspend, because we do it without holding SR_lock.  So you should 
> be able to move the self check and self suspend to before the SR_lock. 
> This would just be a performance optimization.

I would prefer not to skip the resume checks even if they are racy. The 
logic is very obvious in the current form: safepoint for "other" thread 
else self-suspend.

Thanks,
David


> dl
> 
> On 1/24/19 6:46 PM, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8217618
>> webrev: http://cr.openjdk.java.net/~dholmes/8217618/webrev/
>>
>> Lots of analysis in the bug report. Bottom line: SuspendThread of the 
>> current thread was not actually suspending the thread until it hit 
>> specific thread-state transitions. That meant that SuspendThread would 
>> actually return and continue executing native code whilst suspended, 
>> in violation of the specification for it.
>>
>> The fix is quite simple: in java_suspend() we check for the current 
>> thread and call java_suspend_self().
>>
>> Testing:
>>  - Any test that looked like it referred to thread suspension
>>   - hotspot/jtreg/vmTestbase/nsk/jvmti/*
>>   - jdk/
>>      com/sun/jdi/*
>>      java/lang/ThreadGroup/Suspend.java
>> java/lang/management/CompositeData/ThreadInfoCompositeData.java
>>      java/lang/management/ThreadMXBean/*
>>      java/nio/channels/SocketChannel/SendUrgentData.java
>> java/util/logging/LogManager/Configuration/TestConfigurationLock.java
>>
>>  - Mach 5 tiers 1-3 (in progress)
>>
>> Two tests were found to be erroneously relying on SuspendThread 
>> returning whilst suspended:
>>
>> - vmTestbase/nsk/jvmti/scenarios/sampling/SP05/sp05t003/sp05t003.cpp
>>
>> The test updated a shared counter after the SuspendThread call, but it 
>> needed to be updated before the call.
>>
>> - vmTestbase/nsk/jvmti/scenarios/hotswap/HS202/hs202t002/hs202t002.cpp
>>
>> The test was using a 0 return value from SuspendThread as an indicator 
>> that the thread was in the suspended state - but that value can't be 
>> seen until after SuspendThread returns, which is after the thread is 
>> resumed. So I ripped out the custom state tracking logic and replaced 
>> with a simple check of GetThreadState.
>>
>> Thanks,
>> David
>