Looks good! Thanks, /Staffan
On 30 okt 2013, at 12:27, Jaroslav Bachorik <[email protected]> wrote: > Could I have a reviewer to take a look at this, please? > > Thanks, > > -JB- > > On 23.10.2013 18:18, Daniel Fuchs wrote: >> Looks good to me! >> >> -- daniel >> >> On 10/23/13 6:13 PM, Jaroslav Bachorik wrote: >>> On 23.10.2013 17:49, Daniel Fuchs wrote: >>>> On 10/23/13 5:35 PM, Jaroslav Bachorik wrote: >>>>> On 23.10.2013 17:32, Daniel Fuchs wrote: >>>>>> Hi Jaroslav, >>>>>> >>>>>> Have you considered setting a limit on the number of tries you >>>>>> will attempt, rather than just 'while (true)'? >>>>>> It might avoid creating a busy while loop in case there's some global >>>>>> networking issue. >>>>> >>>>> It might be difficult to come up with the right number. Probably a few >>>>> hundred could be enough even for cases when you hit a contiguous range >>>>> of ports occupied by different applications. >>>> >>>> Instead of using an arbitrary 12424 as the default starting point >>>> it could be interesting to try with the first available port + 1; >>>> >>>> something like: >>>> >>>> final int startPort; >>>> try (ServerSocket s = new ServerSocket(0)) { >>>> startPort = s.getLocalPort() + 1; >>>> } >>>> System.out.println("startPort: "+startPort); >>>> >>>> But maybe that's what the test that calls the code is already doing? >>> >>> Nope, it's hardcoded. >>> >>> I've added the cap for the number of consecutive checks for a port being >>> available and dynamic lookup of the first available port (if it's not >>> specified by the system property "rmi.port"). >>> >>> http://cr.openjdk.java.net/~jbachorik/8027058/webrev.01 >>> >>> Thanks for the additional ideas! >>> >>> -JB- >>> >>> >>>> >>>> -- daniel >>>> >>>> >>>>> >>>>> -JB- >>>>> >>>>>> >>>>>> -- daniel >>>>>> >>>>>> On 10/23/13 5:16 PM, Jaroslav Bachorik wrote: >>>>>>> Please, review the following test change: >>>>>>> >>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8027058 >>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8027058/webrev.00 >>>>>>> >>>>>>> The test fails when the requested port is unavailable. The test logic >>>>>>> already runs the subtests with different port numbers (increasing the >>>>>>> port number for each subtest) and this patch adds the logic to >>>>>>> increase >>>>>>> the port number in case of the subtest failure caused by >>>>>>> BindException. >>>>>>> This way the test is able to "autodiscover" free ports and run the >>>>>>> bootstrap subtests successfully. >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>> -JB- >>>>>> >>>>> >>>> >>> >> >
