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

[serguei.spitsyn at oracle.com]

+1

Thanks,
Serguei

On 8/27/18 13:04, Chris Plummer wrote:
> +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();
>>>>>>>       }
>>>>>>>   }
>>>>>>>
>>>>>
>>>
>