RFR: 8282952: Thread::exit should be immune to Thread.stop

[David Holmes]

On Mon, 21 Mar 2022 21:32:42 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> To allow Thread::exit to be immune from asynchronous exceptions causing corruption of state we introduce a simple async exception delivery deferral mechanism. Note that we still allow async exceptions to become the "pending async exception" so they are not lost (potentially desirable in other usecases), but we defer the delivery of that exception i.e when it is moved from being the "pending async exception" to being the "current pending exception" (which is the exception actually in the process of being propagated).
>> 
>> Testing:
>>  - runtime/Thread/StopAtExit.java combined with logging and debugging hooks to show that exceptions were being deferred
>>  - tiers 1-3
>> 
>> Thanks,
>> David
>
> src/hotspot/share/runtime/thread.cpp line 1380:
> 
>> 1378:       // implementation specific clean-up by calling Thread.exit(). We prevent any
>> 1379:       // further asynchronous exceptions from being delivered to ensure the clean-up
>> 1380:       // is not corrupted.
> 
> The last sentence is troublesome... Perhaps:
> 
> 
> ...  We prevent any
> asynchronous exceptions from being delivered while in Thread.exit()
> to ensure the clean-up is not corrupted.
> 
> 
> I dropped "further" and tried to bound the delivery blockage
> to the Thread.exit() call only since the async exception could
> be delivered after the `_no_async` destructor runs.

Okay.

> src/hotspot/share/runtime/thread.cpp line 1617:
> 
>> 1615: 
>> 1616:   if (!clear_async_exception_condition()) {
>> 1617:     if ((_suspend_flags & _async_delivery_disabled) != 0) {
> 
> Hmmm... @robehn is going to want to know about a new use of `_suspend_flags`.

Yes. If we stop using them for async exceptions then the delivery_disabled bit will also have to be relocated. For now at least it is free to use.

> src/hotspot/share/runtime/thread.inline.hpp line 127:
> 
>> 125: inline bool JavaThread::clear_async_exception_condition() {
>> 126:   bool ret = has_async_exception_condition();
>> 127:   clear_suspend_flag(_has_async_exception);
> 
> I gotta wonder why the baseline code always called `clear_suspend_flag()`
> and did that have any bad side effects?

Clearing a cleared bit has no affect and in baseline we are always going to deliver the pending_async_exception, so if it is set then it must be cleared. Now, if delivery is deferred, we must not clear the bit else we lose the fact we may still have to deliver at some point.

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

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