Hi Eric,
I've taken a look at the webrev and I mainly agree with the
changes/refactoring.
On the issue of increasing the timeout to 20 secs; there is a clear race
in this test, and your refactoring has reduced it as much as possible.
Since the sleep will always occur, I would prefer to reduce it from 20
secs to 5 secs. That, in combination with your use of a latch, should be
sufficient.
Also, there is a variant of each test that runs with a 5,000 millisecond
timeout. This timeout should be increased to 20,000 milliseconds.
Otherwise, the blocking IO call may timeout rather than be interrupted (
which is what the test is testing for ).
If you address the above issues, I will be happy to sponsor this change
for you.
-Chris.
On 28/03/14 05:54, Eric Wang wrote:
Hi Chris & All,
Sorry that the fix spent a bit long time for internal review.
Can you please review the fix below for bug JDK-8025209
<https://bugs.openjdk.java.net/browse/JDK-8025209>. The fix is just to
make the thread synchronization is more robust and made some cleanup.
http://cr.openjdk.java.net/~ewang/JDK-8025209/webrev.00/
Thanks,
Eric
On 2014/3/5 18:01, Chris Hegarty wrote:
On 5 Mar 2014, at 09:48, Eric Wang<yiming.w...@oracle.com> wrote:
Hi everyone,
Hi Everyone,
I'm working on the test bughttps://bugs.openjdk.java.net/browse/JDK-8025209,
The test uses Thread.sleep to sync-up threads which is not a correct
assumption. The fix is just to sync-up 2 threads using proper way.
The webrev will be sent after internal review. Please let me know if you have
any comment.
This test is part of the OpenJDK source. Please just post your webrev
externally for review.
Thanks,
-Chris.
Thanks,
Eric