RFR: 8254263: Remove special_runtime_exit_condition() check from ~ThreadInVMForHandshake()

[Richard Reingruber]

On Mon, 12 Oct 2020 20:20:08 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>>> > I meant the block could be removed if we put
>>> > ThreadInVMfromJava/ThreadInVMfromJava (TIVFJ) into
>>> > handle_polling_page_exception(). In that case the thread would notice and honor
>>> > the suspend when leaving the TIVFJ block. This would not be too late, would it?
>>> 
>> 
>>> Yes, it will honor the first suspend in the TIVFJ destructor by calling
>>> handle_special_runtime_exit_condition(), but even in that case, we would still
>>> need the check in SS:block() because of the last call to
>>> SafepointMechanism::process_if_requested(thread()) in
>>> java_suspend_self_with_safepoint_check(). That could be a new
>>> VM_ThreadSuspend() operation to suspend the thread again.
>> 
>> Sorry, I have to ask again to really understand this myself. If it is a new
>> VM_ThreadSuspend() that started while the target thread T was still
>> _thread_blocked in java_suspend_self_with_safepoint_check(), then T can not
>> leave the do-while-loop because is_external_suspend() will return true as
>> setting the suspend flag for T cannot be reordered with the begin of the
>> safepoint.
>> 
>> Also if T could leave the loop then the safepoint could be ended before T
>> reaches SS::block(). It would escape then to java even though it was suspended.
>> 
>> See also review thread for JDK-8252521:
>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-September/041632.html
>
>> > > I meant the block could be removed if we put
>> > > ThreadInVMfromJava/ThreadInVMfromJava (TIVFJ) into
>> > > handle_polling_page_exception(). In that case the thread would notice and honor
>> > > the suspend when leaving the TIVFJ block. This would not be too late, would it?
>> 
>> > Yes, it will honor the first suspend in the TIVFJ destructor by calling
>> > handle_special_runtime_exit_condition(), but even in that case, we would still
>> > need the check in SS:block() because of the last call to
>> > SafepointMechanism::process_if_requested(thread()) in
>> > java_suspend_self_with_safepoint_check(). That could be a new
>> > VM_ThreadSuspend() operation to suspend the thread again.
>> 
>> Sorry, I have to ask again to really understand this myself. If it is a new
>> VM_ThreadSuspend() that started while the target thread T was still
>> _thread_blocked in java_suspend_self_with_safepoint_check(), then T can not
>> leave the do-while-loop because is_external_suspend() will return true as
>> setting the suspend flag for T cannot be reordered with the begin of the
>> safepoint.
> The new JVM_SuspendThread() call (up to VM_ThreadSuspend()) is started after the thread sets its state back to
> _thread_in_Java and exits the do-while() loop but before it calls SafepointMechanism::process_if_requested(this). The
> thread will see there is a safepoint pending and will block in SS::block(). If there is no check in SS::block(), after
> the safepoint finishes the thread will return to java_suspend_self_with_safepoint_check(), back to ~ TIVFJ and then
> back to java.
>> Also if T could leave the loop then the safepoint could be ended before T
>> reaches SS::block(). It would escape then to java even though it was suspended.
> If the VMThread doesn't read the thread's state before it is restored back to _thread_in_Java then the safepoint won't
> progress since the thread is unsafe (_thread_in_Java). So it will have to wait for the thread to call SS::block(). If
> the VMThread did read a safe state (_thread_blocked) then it means we will see we were suspended in the do-while() loop
> and suspend.

Understood now. Thanks!

-------------

PR: https://git.openjdk.java.net/jdk/pull/577