RFR: 8286830: ~HandshakeState should not touch oops

[Robbin Ehn]

On Thu, 19 May 2022 19:17:09 GMT, Patricio Chilano Mateo <pchilanomate 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

@dholmes-ora originally the idea was that if a JT is on a ThreadsList it will respond to handshakes. Note that while on the ThreadsList it may participate in safepoints.
Since we also touch oops after "thread is exiting" we cannot stop responding to handshake after this stage. (ensure_join)

At one point I actually had a patch which removed the terminated state all together, and instead used on or off ThreadsList, which I think is the right approach, reducing complexity.

Anyhow a way longer discussion, this seems fine for now @pchilano , thanks!

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

Marked as reviewed by rehn (Reviewer).

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