RFR: 8268902: Testing for threadObj != NULL is unnecessary in handshake

[Daniel D.Daugherty]

On Wed, 16 Jun 2021 16:33:41 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> The handshake code tests if the JavaThread->is_exiting or that the threadObj() is null. Ever since JDK-8244997, once the JavaThread is running, the _threadObj won't be null until JavaThread is destroyed. So testing is_exiting is all you need to do.
>> In gtest, the test JavaThread doesn't create a _threadObj JDK-8215948 so removing this unnecessary test allows writing gtests for handshakes.
>> 
>> Tested with tier1-6.
>
> src/hotspot/share/runtime/handshake.cpp line 629:
> 
>> 627:   assert(_handshakee->threadObj() != NULL,
>> 628:          "null threadobj is impossible because we're handshaking while the thread is being created");
>> 629:   if (_handshakee->is_exiting()) {
> 
> The assert() is racy when it comes before the `_handshakee->is_exiting()` check.
> It is possible for the target thread (`_handshakee`) to set the exiting condition:
> 
> src/hotspot/share/runtime/thread.cpp: JavaThread::exit():
> 
>     set_terminated(_thread_exiting);
> 
> and call:
> 
>   ensure_join(this);
> 
> which clears the `_threadObj` field while the caller has just
> called `HandshakeState::suspend_with_handshake()`.
> The caller would then fail the assert() when racing with the
> exiting thread when what you want it to do is detect the
> exiting condition and return false.

Update: After re-reading more of the code, I think I have to change
my comment here. The code above (`HandshakeState::suspend_with_handshake()`)
is called from SuspendThreadHandshake::do_thread() which is a synchronous
handshake. The caller requesting the synchronous handshake cannot proceed until
the target thread is handshakeable. When the target thread is in `JavaThread::exit()`
it is in thread state `_thread_in_vm` and that's not a safe/handshakeable state so
the calling thread must wait for the target thread to become handshakeable or for
the exiting condition to be true. No race here is possible.

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

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