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

[Alex Menkov]

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();
>>>>>       }
>>>>>   }
>>>>>
>>>
>