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

[taras ledkov]

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)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>>
>

-- 
With best regards,
Taras Ledkov
Mail-To: taras.ledkov at oracle.com
skype: taras_ledkov
Phone: 7(812)3346-157