Review request for 7195249: Some jtreg tests use hard coded ports

[Yekaterina Kantserova]

Taras,

I agree with the previous comments, that Utils.getFreePort() does not 
guarantee the port will be still free when you start your process. 
Unfortunately I don't think the library can do more. However, there is a 
solution.

Please, look at the *jdk/test/sun/tools/jstatd/JstatdTest.java 
tryToSetupJstatdProcess()*. In brief, the test will try to start a 
process with a free port and then check if 
/java.rmi.server.ExportException: Port already in use/ has been thrown. 
If yes, you have to retry.

Thanks,
Katja



On 12/02/2013 01:39 PM, taras ledkov wrote:
> Hi Everyone,
>
> Whatever logic is to be chosen to select a free port, it is the 
> library responsibility to implements it, would not you agree?
>
> Hence what I am suggesting is to integrate the tests as is.
>
> Should we decide to replace logic of the port selection, we could do 
> it later in the library.
>
> On 21.11.2013 15:00, Jaroslav Bachorik wrote:
>> On 20.11.2013 18:38, Dmitry Samersoff wrote:
>>> Roger,
>>>
>>> As soon as we close a socket nobody can guarantee that the port is 
>>> free.
>>>
>>> Moreover, port returned by getFreePort()[1] remains not accessible for
>>> some time - it depends to system setup, take a look to discussions
>>> around SO_REUSEPORT for Linux or SO_REUSEADDR and SO_LINGER for BSD.
>>>
>>> So from stability point of view it's better to just return random 
>>> number
>>> between 49152 and 65535.
>>
>> Well, this doesn't seem to improve the odds by much. When there are more
>> tests run in parallel, all of them requiring a free port, nothing
>> prevents the random function to return the same port to all of them.
>> Also, two subsequent requests can return the same port and cause
>> problems with timing when a port used by a previous test is not fully
>> ready to be assigned to a different socket. And as Dmitry pointed out
>> unless one can keep hold of the allocated socket and use it later there
>> is no guarantee that a port which was tested unallocated will remain
>> unallocated also for the next few milliseconds.
>>
>> The only fail proof solution would be a port allocating service provided
>> by the harness. Until then we can only (hopefully) decrease the chance
>> of intermittent failures due to a port being in use.
>>
>> -JB-
>>
>>>
>>> -Dmitry
>>>
>>>
>>> [1]
>>>
>>> 141     public static int getFreePort() throws InterruptedException,
>>> IOException {
>>>   142         int port = -1;
>>>   143
>>>   144         while (port <= 0) {
>>>   145             Thread.sleep(100);
>>>   146
>>>   147             ServerSocket serverSocket = null;
>>>   148             try {
>>>   149                 serverSocket = new ServerSocket(0);
>>>   150                 port = serverSocket.getLocalPort();
>>>   151             } finally {
>>>   152                 serverSocket.close();
>>>   153             }
>>>   154         }
>>>   155
>>>   156         return port;
>>>   157     }
>>>   158
>>>
>>>
>>> On 2013-11-20 19:40, roger riggs wrote:
>>>> Hi,
>>>>
>>>> fyi,  The jdk.testlibrary.Utils.getFreePort() method will Open an free
>>>> Socket, close it and return
>>>> the port number.
>>>>
>>>> And as Alan recommended, use (0) when possible to have the system 
>>>> assign
>>>> the port #.
>>>>
>>>> Roger
>>>>
>>>> On 11/20/2013 8:04 AM, Dmitry Samersoff wrote:
>>>>> Taras,
>>>>>
>>>>> *The only* correct way to take really free port is:
>>>>>
>>>>> 1. Chose random number between 49152 and 65535
>>>>> 2. Open socket
>>>>>
>>>>> if socket fails - repeat step 1
>>>>> if socket OK - return *socket*
>>>>>
>>>>>
>>>>> If you can't keep the socket open (e.g. you have to pass port 
>>>>> number as
>>>>> property value) you shouldn't do any pre-check as it has no value 
>>>>> - as
>>>>> as soon as you close socket someone can take the port.
>>>>>
>>>>> So just choose a random number within the range above and let
>>>>> networking
>>>>> code opening socket to handle port conflict.
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>>
>>>>> On 2013-11-20 15:54, taras ledkov wrote:
>>>>>> Hi Everyone,
>>>>>>
>>>>>> I am working on bug 
>>>>>> https://bugs.openjdk.java.net/browse/JDK-7195249.
>>>>>>
>>>>>> There are two webrevs:
>>>>>> Webrev for jdk part:
>>>>>> http://cr.openjdk.java.net/~anazarov/7195249/jdk/webrev.00/
>>>>>>
>>>>>> Webrev for hs part:
>>>>>> http://cr.openjdk.java.net/~anazarov/7195249/hs/webrev.00/
>>>>>>
>>>>>> Please take a look at some notes:
>>>>>> - After discussing with Yekaterina Kantserova & Jaroslav Bachorik 
>>>>>> some
>>>>>> shell tests have been converted to java based tests
>>>>>>
>>>>>> - PasswordFilePermissionTest & SSLConfigFilePermissionTest tests
>>>>>> looked
>>>>>> very similar, so a common parent class was created for them:
>>>>>> AbstractFilePermissionTest
>>>>>>
>>>>>> - What was called RmiRegistrySslTest.java I've renamed to
>>>>>> RmiRegistrySslTestApp.java. The java code to replace old shell 
>>>>>> script
>>>>>> RmiRegistrySslTest.sh is called RmiRegistrySslTest.java, hence the
>>>>>> huge
>>>>>> diff.
>>>>>>
>>>>>> - The new RmiRegistrySslTest.java has some lines similar to the
>>>>>> AbstractFilePermissionTest.java, I nevertheless decided to not
>>>>>> complicate the code further and leave it as is. Please let me 
>>>>>> know if
>>>>>> this is somehow not acceptable
>>>>>>
>>>>>> - com/oracle/java/testlibrary/Utils.java that is added to hotspot
>>>>>> repository is taken from this patch:
>>>>>> http://cr.openjdk.java.net/~ykantser/8023138/webrev.00/test/lib/testlibrary/jdk/testlibrary/Utils.java.sdiff.html 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> - These tests will need additional changes when test library process
>>>>>> tools will support command line options inheritance
>>>>>> (http://mail.openjdk.java.net/pipermail/serviceability-dev/2013-November/013235.html) 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20131204/388d4fe9/attachment.html