Milan,
In order to simulate the "Address already in use" scenario I did this:
TcpDnsServer(int port) {
try {
* new ServerSocket(port, 0, InetAddress.getLoopbackAddress());
serverSocket = new ServerSocket(port, 0,
InetAddress.getLoopbackAddress());
}
catch (BindException ex) {
The test failed, instead of being skipped. That's what I saw in the logs:
jtreg.SkippedException: Cannot start TCP server
at TcpTimeout$TcpDnsServer.<init>(TcpTimeout.java:97)
at TcpTimeout.initTest(TcpTimeout.java:71)
at TestBase.run(TestBase.java:48)
at TcpTimeout.main(TcpTimeout.java:47)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at
com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298)
at java.base/java.lang.Thread.run(Thread.java:830)
Caused by: java.net.BindException: Address already in use: bind
at java.base/sun.nio.ch.Net.bind0(Native Method)
at java.base/sun.nio.ch.Net.bind(Net.java:469)
at java.base/sun.nio.ch.Net.bind(Net.java:458)
at java.base/sun.nio.ch.NioSocketImpl.bind(NioSocketImpl.java:643)
at java.base/java.net.ServerSocket.bind(ServerSocket.java:355)
at java.base/java.net.ServerSocket.<init>(ServerSocket.java:241)
at TcpTimeout$TcpDnsServer.<init>(TcpTimeout.java:91)
... 9 more
JavaTest Message: Test threw exception: jtreg.SkippedException
JavaTest Message: shutting down test
JavaTest Message: Problem cleaning up the following threads:
Thread-0
at
java.base@14-internal/java.net.DualStackPlainDatagramSocketImpl.socketReceiveOrPeekData(Native
Method)
at
java.base@14-internal/java.net.DualStackPlainDatagramSocketImpl.peekData(DualStackPlainDatagramSocketImpl.java:113)
at
java.base@14-internal/java.net.DatagramSocket.receive(DatagramSocket.java:746)
at DNSServer.receiveQuery(DNSServer.java:178)
at DNSServer.run(DNSServer.java:119)
What's going on here? It looks like the test didn't call `cleanupTest()` (and
therefore `server.stopServer()`) because the exception was thrown before it had
reached `runTest()`. I think we must address this, otherwise our
jtreg.SkippedException is only half measure.
I thought about how to do that. I'm thinking of going the full way up to
TestBase as I believe it makes the most sense. Having said that, we'd better
ask the original author (cc'ed), Chris Yin, what he thinks about it. Here's
the proposal:
diff -r 53e139e55605 test/jdk/com/sun/jndi/dns/lib/TestBase.java
--- a/test/jdk/com/sun/jndi/dns/lib/TestBase.java Thu Sep 05 14:59:29
2019 +0100
+++ b/test/jdk/com/sun/jndi/dns/lib/TestBase.java Fri Sep 06 14:08:48
2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -44,10 +44,14 @@
* @throws Exception if any exception
*/
public void run(String[] args) throws Exception {
- initRes();
- initTest(args);
- setupTest();
- launch();
+ try {
+ initRes();
+ initTest(args);
+ setupTest();
+ launch();
+ } finally {
+ cleanupTest();
+ }
}
/**
@@ -84,8 +88,6 @@
if (!handleException(e)) {
throw e;
}
- } finally {
- cleanupTest();
}
}
@@ -108,6 +110,11 @@
/**
* Cleanup test.
+ *
+ * This method may be called by the test at any time, including before all
+ * the resources, initialization, and setup have been completed. This might
+ * require careful attention. For example, before closing a resource this
+ * method should check that resource for being {@code null}.
*/
public abstract void cleanupTest();
}
-Pavel
> On 5 Sep 2019, at 11:20, Milan Mimica <milan.mim...@gmail.com> wrote:
>
> On Wed, 4 Sep 2019 at 20:32, Florian Weimer <fwei...@redhat.com> wrote:
>>
>> If you use the initial UDP timeout (one second, I think), the kernel
>> will not complete the TCP handshake if the initial SYN segment is lost
>> because the retransmit delay during the handshake is longer than that.
>
> We could set a higher timeout value, but I would not prefer that.
> After all, 1 second is just default value, and can be changed. If the
> user wants us to give up on DNS query after the specified timeout then
> lets do just that. True, some queries that previously worked might
> start failing, but that is true even for slow socket.read()
> operations.
>
> New webrev: http://cr.openjdk.java.net/~mmimica/8228580/webrev.01/
>
> * simplified test with no thread (nice catch Pavel)
> * set connect timeout and account for it
>
> --
> Milan Mimica
