I am OK with the change that makes this test more robust. 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. Is UnicastRemoteObject.unexportObject() also doing something similar? I see that the ServerSocket inside is closed on TCPTransport.java:280. Is it reliably called?
Even after this bug is resolved, I'd suggest add some logging and rerun this test unchanged multiple times to see what really happened. --Max > On Oct 15, 2019, at 11:04 AM, Hamlin Li <[email protected]> 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 > > On 2019/10/15 10:43 AM, Joe Darcy wrote: >> Hello, >> >> Structurally, I'd prefer an approach that doesn't increase the minimum time >> needed to run the test. >> >> Any timeout-like value used within the test code should be sensitive to the >> jtreg timeout factor, as done in the webrev. >> >> Thanks, >> >> -Joe >> >> On 10/14/2019 7:20 PM, Weijun Wang wrote: >>> +SeanC >>> >>> The wait might unnecessarily increase the test time. Maybe you can do >>> something like this: >>> >>> int timeout = 10; >>> while (timeout > 0) { >>> sleep(one second); >>> verifyPortFree && return; >>> timeout--; >>> } >>> throw new Exception(still not freed); >>> >>> And Sean, back in JDK-8016728 you said "Let's extend it to 1000ms and see >>> how test behaves". So what do you think? >>> >>> Thanks, >>> Max >>> >>>> On Oct 15, 2019, at 10:03 AM, Hamlin Li <[email protected]> wrote: >>>> >>>> Hi, >>>> >>>> The test is failing more frequently, could some help to review it? >>>> >>>> Thank you >>>> >>>> -Hamlin >>>> >>>> On 2019/9/4 11:11 AM, Hamlin Li wrote: >>>>> some background & comment: in most of failures, the "test.timeout.factor" >>>>> is 10.0 or 8.0, so in the test code this factor should be considered in >>>>> rmi operations such unexporting an object. >>>>> >>>>> Thank you >>>>> >>>>> -Hamlin >>>>> >>>>> On 2019/9/4 11:01 AM, Hamlin Li wrote: >>>>>> Hi, >>>>>> >>>>>> Would you please review the following patch? >>>>>> >>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8134599 >>>>>> >>>>>> webrev: http://cr.openjdk.java.net/~mli/8134599/webrev.00/ >>>>>> >>>>>> >>>>>> Thank you >>>>>> >>>>>> -Hamlin >>>>>>
