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