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

[Hamlin Li]

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
>