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

[David Holmes]

On 26/06/2021 8:43 am, David Holmes wrote:
> On 26/06/2021 3:57 am, Coleen Phillimore wrote:
>> In the last pull request on this topic, I was making more general 
>> (mis)statements, but this null threadObj check is only for the case 
>> where we're suspending a thread which already has a threadObj.  This 
>> null check is unnecessary.
> 
> This is probably true, but possibly only because we screen out 
> jni-attaching threads at a higher-level.

I can't quite join all the dots here. If we look at JDWP allThreads 
command, it will call JVM TI GetAllThreads, which uses:

   // enumerate threads (including agent threads)
   ThreadsListEnumerator tle(current_thread, true);

which is implicitly

   ThreadsListEnumerator tle(current_thread, true, true);

from:

   ThreadsListEnumerator(Thread* cur_thread,
                         bool include_jvmti_agent_threads = false,
                         bool include_jni_attaching_threads = true);

so in this case we will return those threads that are still attaching 
and may have no threadObj().

Can such a thread then be used by the JDWP Suspend() command ... it 
means we will get a NULL in the list of jthreads, and the first thing we 
will try to do is call  JVM TI GetThreadLocalStorage - which is 
specified to treat NULL as "use the current thread"! So that seems like 
a bug - but not the bug this PR is concerned about. :)

I'll follow this up with serviceability folk.

Cheers,
David


> Cheers,
> David
> 
>> Tested already with tier1-6 (rerun tier1).
>>
>> -------------
>>
>> Commit messages:
>>   - 8268902: Testing for threadObj != NULL is unnecessary in suspend 
>> handshake
>>
>> Changes: https://git.openjdk.java.net/jdk/pull/4598/files
>>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4598&range=00
>>    Issue: https://bugs.openjdk.java.net/browse/JDK-8268902
>>    Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
>>    Patch: https://git.openjdk.java.net/jdk/pull/4598.diff
>>    Fetch: git fetch https://git.openjdk.java.net/jdk 
>> pull/4598/head:pull/4598
>>
>> PR: https://git.openjdk.java.net/jdk/pull/4598
>>