RFR: JDK-8019927: [TESTBUG] nsk/jvmti/GetThreadInfo/thrinfo001 intermittently fails with 'invalid thread group' when running with JFR

[Chris Plummer]

+1

Chris

On 8/27/18 10:46 AM, Alex Menkov wrote:
> Looks good to me.
>
> --alex
>
> On 08/27/2018 10:30, Gary Adams wrote:
>> You are right! I had used the original thread group and missed the
>> need to refetch getThreadGroup().
>>
>> Revised changeset:
>>
>> diff --git 
>> a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001.java 
>> b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001.java
>> --- 
>> a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001.java
>> +++ 
>> b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001.java
>> @@ -63,13 +63,13 @@
>>           try {
>>               t_a.join();
>>           } catch (InterruptedException e) {}
>> +        checkInfo(t_a, t_a.getThreadGroup(), 1);
>>
>>           thrinfo001b t_b = new thrinfo001b();
>>           t_b.setPriority(Thread.MIN_PRIORITY);
>>           t_b.setDaemon(true);
>>           checkInfo(t_b, t_b.getThreadGroup(), 2);
>>           t_b.start();
>> -        checkInfo(t_b, t_b.getThreadGroup(), 2);
>>           try {
>>               t_b.join();
>>           } catch (InterruptedException e) {}
>>
>>
>>
>> On 8/27/18, 1:01 PM, Alex Menkov wrote:
>>>
>>>
>>> On 08/27/2018 05:08, Gary Adams wrote:
>>>> I just tried the suggestion to add the check after thread1 join call
>>>> and it fails with :
>>>>
>>>>   Thread thread1: invalid thread group
>>>>
>>>> If a thread completes and we join the thread, not all information
>>>> of the terminated thread is available.
>>>
>>> Per spec after thread termination both Thread.getThreadGroup() and 
>>> jvmti GetThreadInfo should return null.
>>> IsSameObject should return JNI_TRUE if both objects are NULL.
>>> Could you please investigate which function returns unexpected value.
>>>
>>> --alex
>>>
>>>>
>>>> I think we need to limit the test checking to before starting the
>>>> thread and again while it is running. If the thread is finished,
>>>> we can not perform the checks reliably.
>>>>
>>>> On 8/24/18, 8:11 PM, Alex Menkov wrote:
>>>>> Hi Gary,
>>>>>
>>>>> Removing 1st checkInfo (just after t_b.start()) looks good, but I 
>>>>> don't think 2nd checkInfo call should be dropped.
>>>>> I'd rather added checkInfo for t_a after t_a.join()
>>>>> Then the test will cover 3 cases - before thread is started, while 
>>>>> thread is running (checkInfo from thread.run()) and after thread 
>>>>> termination.
>>>>>
>>>>> --alex
>>>>>
>>>>> On 08/24/2018 05:19, Gary Adams wrote:
>>>>>> David pointed out the flaw in this test a long time ago, but
>>>>>> no one followed through to fix the test. When the test thread
>>>>>> runs to completion, there is no way to get the thread info
>>>>>> for the comparisons being made in checkInfo().
>>>>>>
>>>>>> The test already calls checkInfo before starting the thread
>>>>>> and again in the test thread run method. The change below
>>>>>> will make the thrinfo001a and thrinfo001b tests perform the
>>>>>> same checking. It will avoid the race condition when thrinfo001b
>>>>>> finishes quickly.
>>>>>>
>>>>>>    Issue: https://bugs.openjdk.java.net/browse/JDK-8019927
>>>>>>
>>>>>>
>>>>>> diff --git 
>>>>>> a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001.java 
>>>>>> b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001.java 
>>>>>>
>>>>>> --- 
>>>>>> a/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001.java 
>>>>>>
>>>>>> +++ 
>>>>>> b/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetThreadInfo/thrinfo001.java 
>>>>>>
>>>>>> @@ -69,11 +69,9 @@
>>>>>>           t_b.setDaemon(true);
>>>>>>           checkInfo(t_b, t_b.getThreadGroup(), 2);
>>>>>>           t_b.start();
>>>>>> -        checkInfo(t_b, t_b.getThreadGroup(), 2);
>>>>>>           try {
>>>>>>               t_b.join();
>>>>>>           } catch (InterruptedException e) {}
>>>>>> -        checkInfo(t_b, t_b.getThreadGroup(), 2);
>>>>>>           return getRes();
>>>>>>       }
>>>>>>   }
>>>>>>
>>>>
>>