Hi Daniel,
Thank you for the review. The replies are in-lined below. The new
version of the webrev can be found here:
http://cr.openjdk.java.net/~aefimov/8225430/01
With Best Regards,
Aleksei
On 06/08/2019 18:09, Daniel Fuchs wrote:
Hi Aleksei,
Thanks for taking care of these tests!
http://cr.openjdk.java.net/~aefimov/8225430/00/test/jdk/java/net/Socket/NullHost.java
39 svr.bind(new
InetSocketAddress(InetAddress.getLoopbackAddress(), 0));
maybe it would be worth adding a comment just before this line:
// The client side calls Socket((String) null, ...) which
// resolves to InetAddress.getByName((String)null) which in
// turns will resolve to the loopback address.
Thanks - comment added.
HandleContentTypeWithAttrs.java:
Since the original test had a backlog of 50:
138 serverSocket = new ServerSocket(port, 50);
maybe we should preserve it, and uses the ServerSocket constructor
that takes a backlog number in the new code.
I've removed 50 value because the default value in ServerSocket is also
50. I do not see this value documented in javadoc [1] and it could be
changed in future, so let me return it back to the test code.
But I couldn't see how two-integers ServerSocket constructor can be used
here since it will bind the socket address to
InetAddress.anyLocalAddress():
var ss = new ServerSocket(12345,50);
ss.isBound() returns true
ss.getLocalSocketAddress() returns 0.0.0.0/0.0.0.0:12345
Therefore I've added 50 backlog value to the ServerSocket::bind call
[1]
https://docs.oracle.com/en/java/javase/12/docs/api/java.base/java/net/ServerSocket.html
best regards,
-- daniel
PS: RedirectLimit.java
One trick that's been used before - and that might be of interest
here if this test is observed failing again, is to make the server
side ignore any request that obviously don't come from the
test itself. One way of doing that could be e.g to ignore
(closing the accepted socket and doing as if nothing had been
received) a request whose URI path doesn't match what the
client is supposed to send, and possibly make that URI path
sufficiently unique so that the test is the only one to use it.
I'm not suggesting we should do this here, but something to keep
in mind if tests are observed failing due to rogue connections
from other processes on the machine.
Thanks I will keep that trick in mind and apply it if test fails again.
On 06/08/2019 15:35, Aleks Efimov wrote:
Hi,
Please help to review few test fixes which address intermittent
networking failures:
http://cr.openjdk.java.net/~aefimov/8225430/00
JBS:
https://bugs.openjdk.java.net/browse/JDK-8225430
The following tests have been marked with @intermittent keyword:
java/net/DatagramSocket/ReuseAddressTest.java
java/net/ServerSocket/AcceptCauseFileDescriptorLeak.java
With Best Regards,
Aleksei
