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

[David Holmes]

On 22/06/2021 1:41 am, Coleen Phillimore wrote:
> On 6/21/21 12:28 AM, David Holmes 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.
>> Hi Coleen,
>>
>> What about an attaching thread that hits a handshake when trying to 
>> allocate the java.lang.Thread object?
> 
> The attaching thread can't be handshake because it hasn't been added to 
> the Threads list until threadObj is initialized.  As far as I can tell, 
> you can't handshake a thread that's not on the list.

I'm not sure what code you are looking at but attach_current_thread does:

   // This thread will not do a safepoint check, since it has
   // not been added to the Thread list yet.
   { MutexLocker ml(Threads_lock);
     thread->set_active_handles(JNIHandleBlock::allocate_block());
     Threads::add(thread, daemon);
   }

and the thread is now on the threads-list; then it does:

  thread->allocate_threadObj(thread_group, thread_name, daemon, THREAD);

after that. And that code does:

   instanceHandle thread_oop = ik->allocate_instance_handle(CHECK);

   java_lang_Thread::set_thread(thread_oop(), this);
   java_lang_Thread::set_priority(thread_oop(), NormPriority);
   set_threadObj(thread_oop());

The thread must be on the threads-list and able to participate in 
safepoints because it is going to allocate a Java object. After this 
point it also executes Java code - so again absolutely must be on the 
threads-list already.

David
-----

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