Hi Alex,
It looks good to me.
Thanks,
Serguei
On 1/17/19 12:21 PM, Alex Menkov wrote:
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
