RFR of JDK-8049316: TEST_BUG: java/nio/channels/Selector/Wakeup.java fails in same binary run b19

[Roger Riggs]

Hi Hamlin,


On 11/29/2016 9:09 PM, Hamlin Li wrote:
> 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.
I was thinking that the createSleeper(sel, want, before) method forced 
two cases together that
didn't need to be in the same method and could be separated in the 
newSleeperWantInterrupt and
newSleeperWithInterruptBeforeSelect factory methods.


line 104: the timeout value should be scaled by 
jdk/testlibrary/Utils.adjustTimeout (test.timeout.factor).

>>
>>  - 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.

Otherwise is fine.

Thanks, Roger

>
> 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
>>
>