RFR: 8286830: ~HandshakeState should not touch oops

[Patricio Chilano Mateo]

On Fri, 20 May 2022 04:24:29 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> On 8284632 we added ~HandshakeState to cleanup any potential async operations left in the handshake queue. This could trigger the release of an oophandle from its associated oop-storage when trying to delete an AsyncExceptionHandshake object. Since the exiting JT already executed on_thread_detach() this is forbidden. 
>> To fix this I used the original approach that @dcubed-ojdk posted on the 8284632 fix, which is cleaning up those operations before on_thread_detach() is executed. To make sure that no other async exception operation is added to the queue after that I did two things:
>> - Added a check in install_async_exception() to avoid installing the operation if the target already moved to the _thread_exiting state.
>> - Move the setting of the _thread_exiting state to include the case where JavaThread::exit() is called with destroy_vm=true.
>> 
>> I also added another run to tests StopAtExit.java and SuspendAtExit.java with extra contention on the Threads_lock. That increases the odds of the target being handshake safe while being in the _thread_exiting state which allows testing for more pathological cases.
>> 
>> I was able to reproduce the issue by running StopAtExit.java with -XX:+UseShenandoahGC and verified that the test now passes. I also run mach5 tiers1-3. Will run the upper tiers also and will do an additional pass after that.
>> 
>> Thanks,
>> Patricio
>
> src/hotspot/share/runtime/thread.cpp line 1443:
> 
>> 1441:   // we can just put in the exiting state and it will be correctly handled.
>> 1442:   // Also, no more async exceptions will be added to the queue after this point.
>> 1443:   set_terminated(_thread_exiting);
> 
> I would have expected that cleaning up any async exceptions has to happen after we set the _thread_exiting state; otherwise what happens if another thread is trying to install an async exception on the current thread before we set it?

The ordering of cleaning up the async exceptions in the queue and the write of _thread_exiting doesn't really matter as long as the state remains _thread_in_vm. We install async exceptions inside a synchronous handshake, which can only happen while the target is handshake/safepoint safe. Since moving to a safe state by the target and reading of the state by the handshaker uses acquire/release semantics, the handshaker will already see the _thread_exiting value inside the handshake.

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

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