RFR JDK-8216386: vmTestbase/nsk/jvmti/PopFrame/popframe005/TestDescription.java fails

[Alex Menkov]

Hi Gary,

On 01/17/2019 10:53, Gary Adams wrote:
> I like the fact that test.timeout.factor is applied as a multiplier.
> 
> It's not clear why an upper limit had to be added.

As you noted there 3 cases where Thread.join() is called, but the 
expected behavior is different. objWaiterThr2 and popFrameClsThr are 
expected to exit, but objWaiterThr1 is expected to be alive after join 
(i.e. the call is expected to take the time specified). I don't want to 
increase the test run time (for timeout.factor == 10 it would take 18 
extra seconds), so I restricted the timeout for the case.

> Is that 50 or 5 seconds?

Thank you for the catch. It should be 5 seconds (5000 ms)

Updated webrev:
http://cr.openjdk.java.net/~amenkov/popframe005_wait_time/webrev.02/

--alex

> 
>   148             objWaiterThr1.join(Math.min(WAIT_TIME, 50000));
> 
> Why are the other wait times not limited?
> 
>   136             objWaiterThr2.join(WAIT_TIME);
> 
> ...
> 
>   169                 popFrameClsThr.join(WAIT_TIME);
> 
> 
> If you need to apply the upper limit, then it would be better to apply it
> at the beginning.
> 
>    38     static final long WAIT_TIME = 
> Math.min(Utils.adjustTimeout(2000),5000);
> 
> 
> 
> 
> 
> On 1/16/19, 6:28 PM, Alex Menkov wrote:
>> Hi all,
>>
>> please review a fix for
>> https://bugs.openjdk.java.net/browse/JDK-8216386
>> webrev:
>> http://cr.openjdk.java.net/~amenkov/popframe005_wait_time/webrev/
>>
>> The fix updates WAIT_TIME to depend on test.timeout.factor system 
>> property.
>> WAIT_TIME value is used as argument of Thread.join().
>> For the case when the thread is expected to be alive (i.e. 
>> Thread.join() exits by timeout) the timeout value is restricted by 5 
>> seconds to avoid long run time with big timeout.factor values.
>>
>> --alex
>