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

[David Holmes]

On 23/06/2021 12:39 am, Coleen Phillimore wrote:
> 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.
> 
> I am looking at all the threads we create in the vm like the ServiceThread (look for Threads::add()), but not attach_current_thread in jni.  You're right, we have to make this thread safepoint here.
> I wonder why some threads we create call ThreadGroup.add and some don't.  There's lots of duplicate code that's slightly different for each.

I'm not sure on the "why". Some internal threads (e.g. AttachListener) 
don't call the j.l.Thread constructor and so have to directly call the 
ThreadGroup.add method. I suspect because these are not true Java 
threads that there is some Java-side logic that Thread construction 
performs which is not suitable for these internal threads.

> Unfortunately now I need a fix for JDK-8215948, hopefully without another duplicate copy of this thread creation code so I can write my handshake test.

Okay guess I'd better look at it again then. :) I can determine which 
variant of the j.l.Thread creation logic is suitable and then factor 
that out into a support method like Thread::create_internal_java_thread().

David
-----

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