RFR: 8254263: Remove special_runtime_exit_condition() check from ~ThreadInVMForHandshake() [v2]

[Patricio Chilano Mateo]

On Tue, 13 Oct 2020 19:42:18 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   added async exception check
>
> src/hotspot/share/runtime/safepoint.cpp line 992:
> 
>> 990:     // Process pending operation
>> 991:     SafepointMechanism::process_if_requested(self);
>> 992:     self->check_and_handle_async_exceptions();
> 
> I'm good with the addition of this call to check_and_handle_async_exceptions().
> 
> In particular because of this comment block in:
> 
> src/hotspot/share/runtime/thread.cpp:
> void JavaThread::check_and_handle_async_exceptions(bool check_unsafe_error) {
>   if (has_last_Java_frame() && has_async_condition()) {
>     // If we are at a polling page safepoint (not a poll return)
>     // then we must defer async exception because live registers
>     // will be clobbered by the exception path. Poll return is
>     // ok because the call we a returning from already collides
>     // with exception handling registers and so there is no issue.
>     // (The exception handling path kills call result registers but
>     //  this is ok since the exception kills the result anyway).
> 
> so check_and_handle_async_exceptions() clearly was expecting to
> be called when at a " poll return" which is the block that contains
> this new call. That's all good!
> 
> What I can't quite figure out is how we "lost" the call to
> check_and_handle_async_exceptions() that must have been
> made somehow before. Clearly that function was expecting
> to be called from a "poll return" site and that's what you're
> putting back, but how was that call made before and what
> change got rid of it?

The same problematic execution sequence detailed in 8223572 for the external_suspend case was there for async
exceptions too. So, while the thread was blocked in the handshake a ThreadStop() operation could have been executed.
After returning from the handshake there was no async exception check. We probably never noticed it because the throw
would happen anyways on some later transition back to java (vm->java or native->java).  As to how was that call made
before and what change got rid of it: this code was there before handshakes existed. ThreadStop() was implemented as a
safepoint and we were probably relying on the special_runtime_exit_condition() check at the end of SS::block() (exactly
like the one I'm removing from ~TIVFH and which I'll try to get rid of to eliminate recursions).

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

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