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

Daniel D.Daugherty dcubed at openjdk.java.net
Tue Oct 13 20:57:10 UTC 2020


On Tue, 13 Oct 2020 20:33:01 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> 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).

Makes sense. Thanks!

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

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


More information about the hotspot-runtime-dev mailing list