RFR of JDK-8168975: java/rmi/activation/Activatable tests fail due to "Port already in use" in RMID.restart()

[Hamlin Li]

Hi Roger,

Thank you for reviewing. Modified as you suggested, and pushed.

Thank you
-Hamlin

On 2016/11/19 0:28, Roger Riggs wrote:
> Hi Hamlin,
>
> RMID.java:
>   - import of LocalTime would lexically follow jva.rmi
>
>   - 53-54 typos: "ajust" -> "adjust";  "togeter" -> "together"
>
> RMIDSelectorProvider.java:
>
>  - 121, 125 I would have just used System.out.printf directly without 
> introducing a trivial function.
>
> Looks fine, Roger
>
> Roger
>
>
> On 11/17/2016 1:46 AM, Hamlin Li wrote:
>> Hi Chris, Joe, Roger,
>>
>> Thank you for reviewing. Please check the comments inline.
>>
>> new webrev: http://cr.openjdk.java.net/~mli/8168975/webrev.01/
>>
>>
>> Thank you
>> -Hamlin
>>
>> On 2016/11/17 2:30, joe darcy wrote:
>>> In terms of timeout, please use much shorter timeout, seconds not 
>>> minutes. 
>> In case of timeout value used in RMID.restart() and 
>> RMIDSelectorProvider.inheritedChannel(), I recommend to set it 
>> longer, reason is:
>>
>>   1. in most of runs, the test will not face a "port in use" issue, 
>> because the time window between RMID.start and RMID.restart is small, 
>> so a long timeout value will not impact the test run time;
>>   2. if there is a interloper between RMID.start() and 
>> RMID.restart(), then it's better to wait for a longer time to grab 
>> the expected port again rather than wait shorter time to get a test 
>> failure.
>>
>>> Most JDK regression tests complete in well under 10 seconds and few 
>>> tests should run for as long as a minute. 
>> After set "sun.rmi.transport.tcp.handshakeTimeout=5000", test will 
>> not waste too much time at RMID.lookupSystem, normal tests which use 
>> RMID.start and RMID.restart will finish in about dozens of seconds. I 
>> guess running time is acceptable.
>>> Also, please scale any timeout if a timeout factor retrieved from 
>>> the jtreg environment. This allows the test to behave better on 
>>> heavily loaded or slow machines where the test invoker has requested 
>>> more time.
>> Fixed, scale timeout in RMID.restart and 
>> RMIDSelectorProvider.inheritedChannel based on jtreg time factor.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>>
>>> On 11/16/2016 9:01 AM, Chris Hegarty wrote:
>>>>> On 16 Nov 2016, at 07:36, Hamlin Li <huaming.li at oracle.com> wrote:
>>>>>
>>>>> Would you please review below fix?
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8168975
>>>>> webrev: http://cr.openjdk.java.net/~mli/8168975/webrev.00/
>>>> The approach builds on the mechanism put in for 8168975, and seems
>>>> reasonable.
>>>>
>>>> The system property being used to “pass” the port number is 
>>>> specific to
>>>> the test, and not JDK related so should not be named
>>>> java.nio.channels.spi.SelectorProviderPort.  Maybe something like
>>>> openjdk.test.RMIDSelectorProvider.port ?
>> Fixed, modify as "test.java.rmi.testlibrary.RMIDSelectorProvider.port".
>>>>
>>>> -Chris.
>>>>
>>>>> Root Cause: There is a time window between RMID.start() and 
>>>>> RMID.restart(), interloper can step in and bind to the port used 
>>>>> by RMID in RMID.start().
>>>>>
>>>>> Solution:   Modify RMID.java to use inherited channel when calling 
>>>>> RMID.restart. And extend RMIDSelectorProvider.java to try to bind 
>>>>> to specific port, no matter 0 or none-zero(for RMID.restart) in a 
>>>>> while loop, if timeout just call System.exit to exit rmid and let 
>>>>> test fail.
>>>>>
>>>>>
>>>>> Thank you
>>>>> -Hamlin
>>>
>>
>