Should ~ThreadInVMForHandshake() call handle_special_runtime_exit_condition() ?

[Robbin Ehn]

Hi Richard,

I believe you are correct.

Thanks, Robbin

On 5/7/19 8:29 AM, Reingruber, Richard wrote:
> // Repost adding hotspot-runtime-dev at openjdk.java.net
> 
> Hi,
> 
> there's an execution path, where a java thread T transitions from _thread_in_Java to
> _thread_in_vm and back again using an instance of ThreadInVMForHandshake without checking
> has_special_runtime_exit_condition() and calling handle_special_runtime_exit_condition()
> if true.
> 
> I would consider this a bug. Would you agree?
> 
> What about changing transition_back() like this?
> 
> diff -r 61d0e96a6b2d src/hotspot/share/runtime/interfaceSupport.inline.hpp
> --- a/src/hotspot/share/runtime/interfaceSupport.inline.hpp	Thu May 02 15:46:34 2019 -0700
> +++ b/src/hotspot/share/runtime/interfaceSupport.inline.hpp	Mon May 06 14:30:59 2019 +0200
> @@ -137,6 +137,11 @@
>       SafepointMechanism::block_if_requested(_thread);
>   
>       _thread->set_thread_state(_original_state);
> +
> +    if ((_original_state == _thread_in_Java || _original_state == _thread_in_native) &&
> +        _thread->has_special_runtime_exit_condition()) {
> +      _thread->handle_special_runtime_exit_condition(!_thread->is_at_poll_safepoint());
> +    }
>     }
>   
>    public:
> 
> Without that patch an issue could be that thread T continues executing bytecodes after it was
> suspended by a JVMTI agent like this:
> 
> - Handshake operation H is executed for T
> - T calls ThreadSafepointState::handle_polling_page_exception()
> - T arrives in HandshakeState::process_self_inner()
> - T transitions from _thread_in_Java to _thread_in_vm
> - but too late: the VM thread already claimed H
> - T calls _semaphore.wait_with_safepoint_check()
> - T transitions from _thread_in_vm to _thread_blocked
> - The VM thread completes H and calls _semaphore.signal()
> - Before T can transition back to _thread_in_vm, the VM thread schedules another safepoint and
>    executes VM_ThreadSuspend on behalf of a JVMTI agent that wants to suspend T
> - After the safepoint T transitions back to _thread_in_vm and then to _thread_in_Java.
> - T continues executing an undefined number of bytecodes ...
> 
> Cheers, Richard.
>