On 2.2.2015 23:09, Stuart Marks wrote:
On 2/2/15 2:33 AM, Jaroslav Bachorik wrote:
I've applied your comments and the code is a tad simpler now. I also
ironed out
a few more corner cases. I ran the test 500x in a tight loop and no
failure, yay!
Update: http://cr.openjdk.java.net/~jbachorik/8066708/webrev.04
Hi Jaroslav,
Looks quite a bit more straightforward now. I'm pretty much OK with this
if you're OK with it; I think I've reviewed it enough times already. :-)
I have a couple comments on test_09 that you might want to address
before you push or sometime in the future.
Line 714 typo "hugging" => "hogging"
D'oh. Thanks.
But I'm not convinced this retry logic in the while-loop from lines
716-723 is necessary. If you've already opened a server socket on a
port, I've never seen a case where opening the same port *again* will
succeed, so why bother?
I saw the test failing without this retry logic. The purpose is to make
sure that the port is really taken before the test continues.
I'd suggest simply opening ServerSocket(0) and then getting the port via
getLocalPort(). I've never seen this fail. This case should work since
you actually want to open up some random port, instead of generating a
random port number for somebody else (the subprocess) to open. You can
then allocate one fewer random port. You might want to have a little
loop to check that the random port number isn't a duplicate of the
actual port that you just opened. I think this lets the code boil down a
bit further:
ServerSocket ss = new ServerSocket(0);
try {
int localPort = ss.getLocalPort();
int[] ports;
do {
ports = PortAllocator.allocatePorts(1);
} while (localPort != ports[0]);
AtomicBoolean checks = new AtomicBoolean(false);
...
} finally {
ss.close();
s.stop();
}
I'll give it a try and see how this behaves.
Thanks,
-JB-
s'marks
