Fwd: Re: Review request for 7195249: Some jtreg tests use hard coded ports
Alexandre (Shura) Iline
alexandre.iline at oracle.com
Mon Dec 9 02:02:34 PST 2013
Guys.
Let me try to sum up what was said before and may be suggest a compromise.
1. There is a desire to have a support port allocation on the level of a
JTReg suite execution. Taras created a bug for that
(https://bugs.openjdk.java.net/browse/JDK-7195249). Whether it is a test
harness API or a library API does not really matter from usage point of
view.
2. There is no way to make the tests absolutely stable, whatever port
allocation logic is used. The best we could do is to try to perform the
test logic with different ports until the test succeeds.
Both arguments make sense. #2 is the ultimate answer, of course, but
better be used in conjunction with a meaningful port selection algorithm.
At the same time, copying a loop-until-success login from one test to
another may be not the best solution. Library could help with that I
believe. There only need to be an API method which takes behavior as a
parameter and run it until it succeeds. Something like:
public <T> runOnAFreePort(Function<T, Integer>)
or similar. There could be arguments of how/whether to implement it, the
solution would not work for shell tests, etc, but still ...
With the tests in question though, we have a few options.
1. Integrate tests as is. Get to it later after reaching agreement in
the library, etc.
2. Implement loops in every test.
3. Wait for the library to be ready and only then integrate the changes.
Please let us know which one is closer to your heart.
I personally prefer #1 for the reason that the changes already supposed
to make the tests more stable and also there are many more tests tests
which use ports, so the scope of the problem is bigger than these.
Shura
> 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)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
>
>
More information about the serviceability-dev
mailing list