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

[Roger Riggs]

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