Hi Roger,
Thank you for reviewing, please check comments in line.
webrev: http://cr.openjdk.java.net/~mli/8049316/webrev.01/
On 2016/11/30 0:03, Roger Riggs wrote:
Hi Hamlin,
Wakeup.java:
- Some refactoring may make it easier to understand.
Perhaps moving waitSleeper to be a method on Sleeper.
The use of cyclicBarrier seems fine but could use a comment
somewhere saying what
threads/functions are being coordinated; putting them both in Sleeper
would make it easier to see.
- line 126,127: Add a Sleeper constructor to initialize these as needed.
- Some of the test stimulus is buried in the newSleeper methods that
was previously
in line in main. It seemed clearer what case was being tested in
the original.
They are buried in static factory 'newSleeper' functions with
uninformative names.
lines 112, 116, 120.
All above fixed.
- If I read it right, some cases where events don't happen that used
to be reported as exceptions
("Interrupt never delivered") will now be reported as the test
timing out. (infinite loop at line 85).
No, it will finally checked by a time sleeper.finish(0). At first, I
don't want to depends on a specific time, because I don't know what
exact timeout we should use, but as you asked, I think it's ok to
finish(20_000), it should be long enough.
Thank you
-Hamlin
Thanks, Roger
On 11/29/2016 4:23 AM, Hamlin Li wrote:
Would you please review the below patch?
bug: https://bugs.openjdk.java.net/browse/JDK-8049316
webrev: http://cr.openjdk.java.net/~mli/8049316/webrev.00/
Root cause:
1. it depends on sleeping time to check failure, which is not
reliable in some extreme situation
2. it mix several tests together with a loop in Sleeper
Solution:
1. synchronize between threads.
2. isolate tests.
Thank you
Hamlin