6993490: SocketTimeoutException on HTTP keep-alive connections
Reviewed-by: michaelm
--- a/jdk/src/share/classes/sun/net/NetworkClient.java Thu Oct 21 14:39:58 2010 +0100
+++ b/jdk/src/share/classes/sun/net/NetworkClient.java Thu Oct 21 16:49:36 2010 +0100
@@ -40,6 +40,12 @@
* @author Jonathan Payne
*/
public class NetworkClient {
+ /* Default value of read timeout, if not specified (infinity) */
+ public static final int DEFAULT_READ_TIMEOUT = -1;
+
+ /* Default value of connect timeout, if not specified (infinity) */
+ public static final int DEFAULT_CONNECT_TIMEOUT = -1;
+
protected Proxy proxy = Proxy.NO_PROXY;
/** Socket for communicating with server. */
protected Socket serverSocket = null;
@@ -53,8 +59,8 @@
protected static int defaultSoTimeout;
protected static int defaultConnectTimeout;
- protected int readTimeout = -1;
- protected int connectTimeout = -1;
+ protected int readTimeout = DEFAULT_READ_TIMEOUT;
+ protected int connectTimeout = DEFAULT_CONNECT_TIMEOUT;
/* Name of encoding to use for output */
protected static String encoding;
@@ -71,16 +77,12 @@
return null;
}
});
- if (vals[0] == 0)
- defaultSoTimeout = -1;
- else
+ if (vals[0] != 0) {
defaultSoTimeout = vals[0];
-
- if (vals[1] == 0)
- defaultConnectTimeout = -1;
- else
+ }
+ if (vals[1] != 0) {
defaultConnectTimeout = vals[1];
-
+ }
encoding = encs[0];
try {
@@ -232,7 +234,23 @@
return connectTimeout;
}
+ /**
+ * Sets the read timeout.
+ *
+ * Note: Public URLConnection (and protocol specific implementations)
+ * protect against negative timeout values being set. This implemenation,
+ * and protocol specific implementations, use -1 to represent the default
+ * read timeout.
+ *
+ * This method may be invoked with the default timeout value when the
+ * protocol handler is trying to reset the timeout after doing a
+ * potentially blocking internal operation, e.g. cleaning up unread
+ * response data, buffering error stream response data, etc
+ */
public void setReadTimeout(int timeout) {
+ if (timeout == DEFAULT_READ_TIMEOUT)
+ timeout = defaultSoTimeout;
+
if (serverSocket != null && timeout >= 0) {
try {
serverSocket.setSoTimeout(timeout);
--- a/jdk/src/share/classes/sun/net/www/protocol/ftp/FtpURLConnection.java Thu Oct 21 14:39:58 2010 +0100
+++ b/jdk/src/share/classes/sun/net/www/protocol/ftp/FtpURLConnection.java Thu Oct 21 16:49:36 2010 +0100
@@ -46,6 +46,7 @@
import java.util.StringTokenizer;
import java.util.Iterator;
import java.security.Permission;
+import sun.net.NetworkClient;
import sun.net.www.MessageHeader;
import sun.net.www.MeteredStream;
import sun.net.www.URLConnection;
@@ -102,11 +103,11 @@
static final int BIN = 2;
static final int DIR = 3;
int type = NONE;
- /* Redefine timeouts from java.net.URLConnection as we nee -1 to mean
+ /* Redefine timeouts from java.net.URLConnection as we need -1 to mean
* not set. This is to ensure backward compatibility.
*/
- private int connectTimeout = -1;
- private int readTimeout = -1;
+ private int connectTimeout = NetworkClient.DEFAULT_CONNECT_TIMEOUT;;
+ private int readTimeout = NetworkClient.DEFAULT_READ_TIMEOUT;;
/**
* For FTP URLs we need to have a special InputStream because we
--- a/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java Thu Oct 21 14:39:58 2010 +0100
+++ b/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java Thu Oct 21 16:49:36 2010 +0100
@@ -359,11 +359,11 @@
private TunnelState tunnelState = TunnelState.NONE;
- /* Redefine timeouts from java.net.URLConnection as we nee -1 to mean
+ /* Redefine timeouts from java.net.URLConnection as we need -1 to mean
* not set. This is to ensure backward compatibility.
*/
- private int connectTimeout = -1;
- private int readTimeout = -1;
+ private int connectTimeout = NetworkClient.DEFAULT_CONNECT_TIMEOUT;
+ private int readTimeout = NetworkClient.DEFAULT_READ_TIMEOUT;
/* Logging support */
private static final PlatformLogger logger =
@@ -1041,9 +1041,9 @@
throw new ProtocolException("Server rejected operation");
}
}
- if (oldTimeout > 0) {
- http.setReadTimeout(oldTimeout);
- }
+
+ http.setReadTimeout(oldTimeout);
+
responseCode = -1;
responses.reset();
// Proceed
--- a/jdk/test/sun/net/www/http/HttpClient/B6726695.java Thu Oct 21 14:39:58 2010 +0100
+++ b/jdk/test/sun/net/www/http/HttpClient/B6726695.java Thu Oct 21 16:49:36 2010 +0100
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 6726695
+ * @bug 6726695 6993490
* @summary HttpURLConnection shoul support 'Expect: 100-contimue' headers for PUT
*/
@@ -184,7 +184,15 @@
out.flush();
// Then read the body
char[] cbuf = new char[512];
- int l = in.read(cbuf);
+ in.read(cbuf);
+
+ /* Force the server to not respond for more that the expect 100-Continue
+ * timeout set by the HTTP handler (5000 millis). This ensures the
+ * timeout is correctly resets the default read timeout, infinity.
+ * See 6993490. */
+ System.out.println("server sleeping...");
+ try {Thread.sleep(6000); } catch (InterruptedException e) {}
+
// finally send the 200 OK
out.print("HTTP/1.1 200 OK");
out.print("Server: Sun-Java-System-Web-Server/7.0\r\n");
--- a/jdk/test/sun/net/www/http/KeepAliveCache/B5045306.java Thu Oct 21 14:39:58 2010 +0100
+++ b/jdk/test/sun/net/www/http/KeepAliveCache/B5045306.java Thu Oct 21 16:49:36 2010 +0100
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 5045306 6356004
+ * @bug 5045306 6356004 6993490
* @library ../../httptest/
* @build HttpCallback HttpServer HttpTransaction
* @run main/othervm B5045306
@@ -32,7 +32,6 @@
import java.net.*;
import java.io.*;
-import java.nio.channels.*;
import java.lang.management.*;
/* Part 1:
@@ -164,6 +163,14 @@
failed = true;
trans.setResponseHeader ("Content-length", Integer.toString(0));
+
+ /* Force the server to not respond for more that the timeout
+ * set by the keepalive cleaner (5000 millis). This ensures the
+ * timeout is correctly resets the default read timeout,
+ * infinity. See 6993490. */
+ System.out.println("server sleeping...");
+ try {Thread.sleep(6000); } catch (InterruptedException e) {}
+
trans.sendResponse(200, "OK");
} else if(path.equals("/part2")) {
System.out.println("Call to /part2");
--- a/jdk/test/sun/net/www/protocol/http/ChunkedErrorStream.java Thu Oct 21 14:39:58 2010 +0100
+++ b/jdk/test/sun/net/www/protocol/http/ChunkedErrorStream.java Thu Oct 21 16:49:36 2010 +0100
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 6488669 6595324
+ * @bug 6488669 6595324 6993490
* @run main/othervm ChunkedErrorStream
* @summary Chunked ErrorStream tests
*/
@@ -48,6 +48,18 @@
* 2) Client sends request to server and tries to
* getErrorStream(). 4K + 10 bytes must be read from
* the errorStream.
+ *
+ * Part 3: 6993490
+ * Reuse persistent connection from part 2, the error stream
+ * buffering will have set a reduced timeout on the socket and
+ * tried to reset it to the default, infinity. Client must not
+ * throw a timeout exception. If it does, it indicates that the
+ * default timeout was not reset correctly.
+ * If no timeout exception is thrown, it does not guarantee that
+ * the timeout was reset correctly, as there is a potential race
+ * between the sleeping server and the client thread. Typically,
+ * 1000 millis has been enought to reliable reproduce this problem
+ * since the error stream buffering sets the timeout to 60 millis.
*/
public class ChunkedErrorStream
@@ -75,19 +87,18 @@
} finally {
httpServer.stop(1);
}
-
}
void doClient() {
- for (int times=0; times<2; times++) {
+ for (int times=0; times<3; times++) {
HttpURLConnection uc = null;
try {
InetSocketAddress address = httpServer.getAddress();
String URLStr = "http://localhost:" + address.getPort() + "/test/";
if (times == 0) {
- URLStr += 6488669;
+ URLStr += "first";
} else {
- URLStr += 6595324;
+ URLStr += "second";
}
System.out.println("Trying " + URLStr);
@@ -97,6 +108,11 @@
throw new RuntimeException("Failed: getInputStream should throw and IOException");
} catch (IOException e) {
+ if (e instanceof SocketTimeoutException) {
+ e.printStackTrace();
+ throw new RuntimeException("Failed: SocketTimeoutException should not happen");
+ }
+
// This is what we expect to happen.
InputStream es = uc.getErrorStream();
byte[] ba = new byte[1024];
@@ -112,7 +128,7 @@
if (count == 0)
throw new RuntimeException("Failed: ErrorStream returning 0 bytes");
- if (times == 1 && count != (4096+10))
+ if (times >= 1 && count != (4096+10))
throw new RuntimeException("Failed: ErrorStream returning " + count +
" bytes. Expecting " + (4096+10));
@@ -128,13 +144,13 @@
httpServer = com.sun.net.httpserver.HttpServer.create(new InetSocketAddress(0), 0);
// create HttpServer context
- HttpContext ctx1 = httpServer.createContext("/test/6488669", new Handler6488669());
- HttpContext ctx2 = httpServer.createContext("/test/6595324", new Handler6595324());
+ httpServer.createContext("/test/first", new FirstHandler());
+ httpServer.createContext("/test/second", new SecondHandler());
httpServer.start();
}
- class Handler6488669 implements HttpHandler {
+ class FirstHandler implements HttpHandler {
public void handle(HttpExchange t) throws IOException {
InputStream is = t.getRequestBody();
byte[] ba = new byte[1024];
@@ -156,13 +172,22 @@
}
}
- class Handler6595324 implements HttpHandler {
+ static class SecondHandler implements HttpHandler {
+ /* count greater than 0, slow response */
+ static int count = 0;
+
public void handle(HttpExchange t) throws IOException {
InputStream is = t.getRequestBody();
byte[] ba = new byte[1024];
while (is.read(ba) != -1);
is.close();
+ if (count > 0) {
+ System.out.println("server sleeping...");
+ try { Thread.sleep(1000); } catch(InterruptedException e) {}
+ }
+ count++;
+
t.sendResponseHeaders(404, 0);
OutputStream os = t.getResponseBody();