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

[Daniel D.Daugherty]

On Wed, 16 Jun 2021 16:03:04 GMT, Coleen Phillimore <coleenp 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.

Changes requested by dcubed (Reviewer).

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.

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

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