RFR (M): 8231595 [TEST] develop a test case for SuspendThreadList including current thread

[Chris Plummer]

On 9/30/19 1:21 PM, serguei.spitsyn at oracle.com wrote:
> Hi Chris,
>
> Thank you for reviewing this!
>
>
> On 9/28/19 12:33, Chris Plummer wrote:
>> Hi Serguei,
>>
>> Overall looks good. A few questions:
>>
>> I don't understand the need for all the 'i' and 'n' theatrics in the 
>> shouldFinish loop. Can you explain and also add a comment?
>
> I used this part from one of the old SuspendThreadList nsk tests in 
> the vmTestbase.
> As I understand it, the point was to make sure the JVMTI 
> SuspendThreadList works well
> wen the top frames executed on tested threads have been compiled.
> These code is needed to make the run() method hot.
> I can add a comment if you think it is not clear.
>
>
>>
>> Is this comment right?
>>
>>  193     // set thread is not ready again
>>  194     public void setAllThreadsReady() {
>>  195         allThreadsReady = true;
>>  196     }
>
> Nice catch.
> The comment is not needed anymore.
> Is a leftover from previous test version.
>
>>
>> Also, shouldn't "setAllThreadsReady()" be static?
>
> Right. It has to be static. Will fix it.
>
>
>> Do you think it would be useful to also run the test with the last 
>> thread in the list being the suspender thread?
>
> I'm not sure it is worth to do it.
> It'd add more complexity into the test.
> We could try to make the suspender thread to be random though.
I don't think random is good. Makes it hard to reproduce an issue if it 
turns up. I was thinking just rerun the test for each possible suspender.

thanks,

Chris
>
> Thanks,
> Serguei
>
>>
>> thanks,
>>
>> Chris
>>
>> On 9/27/19 6:25 PM, serguei.spitsyn at oracle.com wrote:
>>> Please, review fix for test enhancement:
>>>   https://bugs.openjdk.java.net/browse/JDK-8231595
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8231595-jvmti-susp-tlist.1/ 
>>>
>>>
>>> Summary:
>>>   New test is a coverage for the JVMTI bug:
>>>     https://bugs.openjdk.java.net/browse/JDK-8217762
>>>       SuspendThreadList won't work correctly if the current thread 
>>> is not last in the list
>>>
>>>   It provides a prove the bug JDK-8217762 does not exist
>>>   as the test is passed with the current implementation.
>>>
>>> Thanks,
>>> Serguei
>>
>>
>