From 0d92e368eebfee9ba4c568fa8bf6a4f05cbc5077 Mon Sep 17 00:00:00 2001 From: Chris Hegarty Date: Thu, 21 Oct 2010 16:49:36 +0100 Subject: [PATCH] 6993490: SocketTimeoutException on HTTP keep-alive connections Reviewed-by: michaelm --- .../share/classes/sun/net/NetworkClient.java | 38 +++++++++++----- .../www/protocol/ftp/FtpURLConnection.java | 7 +-- .../www/protocol/http/HttpURLConnection.java | 12 ++--- .../sun/net/www/http/HttpClient/B6726695.java | 12 ++++- .../net/www/http/KeepAliveCache/B5045306.java | 11 ++++- .../www/protocol/http/ChunkedErrorStream.java | 45 ++++++++++++++----- 6 files changed, 92 insertions(+), 33 deletions(-) diff --git a/jdk/src/share/classes/sun/net/NetworkClient.java b/jdk/src/share/classes/sun/net/NetworkClient.java index 01322c91acc..1e88dc884c4 100644 --- a/jdk/src/share/classes/sun/net/NetworkClient.java +++ b/jdk/src/share/classes/sun/net/NetworkClient.java @@ -40,6 +40,12 @@ import java.security.PrivilegedAction; * @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 @@ public class NetworkClient { 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 @@ public class NetworkClient { 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 @@ public class NetworkClient { 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); diff --git a/jdk/src/share/classes/sun/net/www/protocol/ftp/FtpURLConnection.java b/jdk/src/share/classes/sun/net/www/protocol/ftp/FtpURLConnection.java index 49af37825a2..826f1afc5ee 100644 --- a/jdk/src/share/classes/sun/net/www/protocol/ftp/FtpURLConnection.java +++ b/jdk/src/share/classes/sun/net/www/protocol/ftp/FtpURLConnection.java @@ -46,6 +46,7 @@ import java.net.ProxySelector; 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 @@ public class FtpURLConnection extends URLConnection { 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 diff --git a/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java b/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java index 56ed6a40fa3..69f12a5c601 100644 --- a/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java +++ b/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java @@ -359,11 +359,11 @@ public class HttpURLConnection extends java.net.HttpURLConnection { 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 @@ public class HttpURLConnection extends java.net.HttpURLConnection { throw new ProtocolException("Server rejected operation"); } } - if (oldTimeout > 0) { - http.setReadTimeout(oldTimeout); - } + + http.setReadTimeout(oldTimeout); + responseCode = -1; responses.reset(); // Proceed diff --git a/jdk/test/sun/net/www/http/HttpClient/B6726695.java b/jdk/test/sun/net/www/http/HttpClient/B6726695.java index 9e69e8e3d46..01840850cb0 100644 --- a/jdk/test/sun/net/www/http/HttpClient/B6726695.java +++ b/jdk/test/sun/net/www/http/HttpClient/B6726695.java @@ -23,7 +23,7 @@ /* * @test - * @bug 6726695 + * @bug 6726695 6993490 * @summary HttpURLConnection shoul support 'Expect: 100-contimue' headers for PUT */ @@ -184,7 +184,15 @@ public class B6726695 extends Thread { 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"); diff --git a/jdk/test/sun/net/www/http/KeepAliveCache/B5045306.java b/jdk/test/sun/net/www/http/KeepAliveCache/B5045306.java index ac9d4e568f3..08b3079b5cf 100644 --- a/jdk/test/sun/net/www/http/KeepAliveCache/B5045306.java +++ b/jdk/test/sun/net/www/http/KeepAliveCache/B5045306.java @@ -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 @@ class SimpleHttpTransaction implements HttpCallback 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"); diff --git a/jdk/test/sun/net/www/protocol/http/ChunkedErrorStream.java b/jdk/test/sun/net/www/protocol/http/ChunkedErrorStream.java index ee5368a59cc..d5e72f44889 100644 --- a/jdk/test/sun/net/www/protocol/http/ChunkedErrorStream.java +++ b/jdk/test/sun/net/www/protocol/http/ChunkedErrorStream.java @@ -23,7 +23,7 @@ /* * @test - * @bug 6488669 6595324 + * @bug 6488669 6595324 6993490 * @run main/othervm ChunkedErrorStream * @summary Chunked ErrorStream tests */ @@ -48,6 +48,18 @@ import com.sun.net.httpserver.*; * 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 @@ public class ChunkedErrorStream } 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 @@ public class ChunkedErrorStream 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 @@ public class ChunkedErrorStream 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 @@ public class ChunkedErrorStream 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 @@ public class ChunkedErrorStream } } - 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();