> On Oct 15, 2019, at 5:44 PM, Hamlin Li <huaming...@oracle.com> wrote:
> 
> 
> On 2019/10/15 2:44 PM, Weijun Wang wrote:
>> I am OK with the change that makes this test more robust.
> Thanks Max.
>> 
>> However, I am not an expert in RMI and don't know why the port cannot be 
>> released. If verifyPortFree(PORT) on line 60 can always succeed right after 
>> TestLibrary.getUnusedRandomPort() tries to create a ServerSocket at PORT and 
>> close it, this means the OS underneath spends no time in freeing the port.
> 
> No, the port is not freed here, it's only freed by unexportObject.

I meant the auto close inside getUnusedRandomPort().

> 
> But the original code is a little bit faulty at verifyPortInUse, it should be 
> as below, I have also updated the webrev in place: 
> http://cr.openjdk.java.net/~mli/8134599/webrev.01/
> 
> @@ -101,6 +112,7 @@
>      private static void verifyPortInUse(int port) throws IOException {
>          try {
>              verifyPortFree(port);
> +            throw new RuntimeException("port is not in use: " + port);
> 
>> Is UnicastRemoteObject.unexportObject() also doing something similar? I see 
>> that the ServerSocket inside is closed on TCPTransport.java:280. Is it 
>> reliably called?
> 
> In my understanding, it's not a sync operation, it's async, that means when 
> unexportObject returns, it just triggers the port release, does not mean it 
> already released the port.

Ah.... 

> 
>> 
>> Even after this bug is resolved, I'd suggest add some logging and rerun this 
>> test unchanged multiple times to see what really happened.
> 
> Unfortunately, most of time this kind of issue is not easy to reproduce by 
> running it multiple times. In my point of view, current logging is sufficient 
> for diagnosing.

I see some logging calls inside source code. Maybe you can set a logging config 
file to let it show more?

--Max

> 
> Thank you
> 
> -Hamlin
> 
>> 
>> --Max
>> 
>> 
>>> On Oct 15, 2019, at 11:04 AM, Hamlin Li <huaming...@oracle.com> wrote:
>>> 
>>> Thanks for reviewing, I updated change at: 
>>> http://cr.openjdk.java.net/~mli/8134599/webrev.01/
>>> 
>>> it does not increase minimum time time and consider timeout factor at the 
>>> same time.
>>> 
>>> Thank you
>>> 
>>> -Hamlin
>>> 

Reply via email to