On Wed, 7 Apr 2021 14:33:00 GMT, Robbin Ehn <[email protected]> wrote:
>> test/hotspot/jtreg/serviceability/jvmti/SuspendWithCurrentThread/SuspendWithCurrentThread.java
>> line 132:
>>
>>> 130: ThreadToSuspend.setAllThreadsReady();
>>> 131:
>>> 132: while (!checkSuspendedStatus()) {
>>
>> The changes to this test are still quite unclear to me. Too hard to figure
>> out what the test was originally trying to do!
>
> The test worked because we didn't check for suspend when leaving a safepoint
> request back to native.
> @pchilano have more info on why the test even works today.
Right, the reason the test was working so far but not anymore without this
changes is because before we didn't check for suspension on ~ThreadBlockInVM()
after leaving a safepoint safe state. The interaction is kind of subtle:
In the test the main thread creates 10 working threads and sets one of those to
have the role of 'Suspender'. The 'Suspender' acquires agent_monitor(a
jvmtirawmonitor), suspends all working threads including itself by calling
jvmti->SuspendThreadList() and then it releases agent_monitor. In the mean time
the main thread just blocks on agent_monitor until 'Suspender' releases it to
check that everybody was indeed suspended. Now, at first glance this looks
like(at least for me) the test should deadlock because 'Suspender' should never
return from SuspendThreadList() to release agent_monitor since he was suspended
too. The reason 'Suspender' returns from SuspendThreadList() is because he
never actually checks for suspension after the safepoint operation
(VM_ThreadsSuspendJVMTI) finished. 'Suspender' waits on the VMOperation_lock
with as_suspend_equivalent=false (default) so it transitions back to
_thread_in_vm without suspending. And then when going back to native in
~ThreadInVMfromNati
ve() we also don't check for suspends (we now check it here too actually).
The JVMTI specs says this about SuspendThreadList:
"If the calling thread is specified in the request_list array, this function
will not return until some other thread resumes it."
So this patch actually fixes that.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3191
