# HG changeset patch # User chegar # Date 1533810192 -3600 # Node ID 31d9e82b2e6487fb81747f6ab29d746d186c2e12 # Parent a6fa2016cff109b565bee4e700b5061892b3885a 8208391: Differentiate response and connect timeouts in HTTP Client API Reviewed-by: michaelm diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/java/net/http/HttpClient.java --- a/src/java.net.http/share/classes/java/net/http/HttpClient.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/java/net/http/HttpClient.java Thu Aug 09 11:23:12 2018 +0100 @@ -34,6 +34,7 @@ import java.net.URLPermission; import java.security.AccessController; import java.security.PrivilegedAction; +import java.time.Duration; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; @@ -84,6 +85,7 @@ *
{@code HttpClient client = HttpClient.newBuilder() * .version(Version.HTTP_1_1) * .followRedirects(Redirect.NORMAL) + * .connectTimeout(Duration.ofSeconds(20)) * .proxy(ProxySelector.of(new InetSocketAddress("proxy.example.com", 80))) * .authenticator(Authenticator.getDefault()) * .build(); @@ -94,7 +96,7 @@ *Asynchronous Example *
{@code HttpRequest request = HttpRequest.newBuilder() * .uri(URI.create("https://foo.com/")) - * .timeout(Duration.ofMinutes(1)) + * .timeout(Duration.ofMinutes(2)) * .header("Content-Type", "application/json") * .POST(BodyPublishers.ofFile(Paths.get("file.json"))) * .build(); @@ -197,6 +199,26 @@ public Builder cookieHandler(CookieHandler cookieHandler); /** + * Sets the connect timeout duration for this client. + * + *In the case where a new connection needs to be established, if + * the connection cannot be established within the given {@code + * duration}, then {@link HttpClient#send(HttpRequest,BodyHandler) + * HttpClient::send} throws an {@link HttpConnectTimeoutException}, or + * {@link HttpClient#sendAsync(HttpRequest,BodyHandler) + * HttpClient::sendAsync} completes exceptionally with an + * {@code HttpConnectTimeoutException}. If a new connection does not + * need to be established, for example if a connection can be reused + * from a previous request, then this timeout duration has no effect. + * + * @param duration the duration to allow the underlying connection to be + * established + * @return this builder + * @throws IllegalArgumentException if the duration is non-positive + */ + public Builder connectTimeout(Duration duration); + + /** * Sets an {@code SSLContext}. * *
If this method is not invoked prior to {@linkplain #build() @@ -345,6 +367,17 @@ public abstract Optional
cookieHandler(); /** + * Returns an {@code Optional} containing the connect timeout duration + * for this client. If the {@linkplain Builder#connectTimeout(Duration) + * connect timeout duration} was not set in the client's builder, then the + * {@code Optional} is empty. + * + * @return an {@code Optional} containing this client's connect timeout + * duration + */ + public abstract Optional connectTimeout(); + + /** * Returns the follow redirects policy for this client. The default value * for client's built by builders that do not specify a redirect policy is * {@link HttpClient.Redirect#NEVER NEVER}. diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/java/net/http/HttpConnectTimeoutException.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/java.net.http/share/classes/java/net/http/HttpConnectTimeoutException.java Thu Aug 09 11:23:12 2018 +0100 @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2018, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package java.net.http; + +/** + * Thrown when a connection, over which an {@code HttpRequest} is intended to be + * sent, is not successfully established within a specified time period. + * + * @since 11 + */ +public class HttpConnectTimeoutException extends HttpTimeoutException { + + private static final long serialVersionUID = 321L + 11L; + + /** + * Constructs an {@code HttpConnectTimeoutException} with the given detail + * message. + * + * @param message + * The detail message; can be {@code null} + */ + public HttpConnectTimeoutException(String message) { + super(message); + } +} diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/AbstractAsyncSSLConnection.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/AbstractAsyncSSLConnection.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/AbstractAsyncSSLConnection.java Thu Aug 09 11:23:12 2018 +0100 @@ -80,11 +80,9 @@ engine = createEngine(context, serverName.getName(), port, sslParameters); } - abstract HttpConnection plainConnection(); abstract SSLTube getConnectionFlow(); final CompletableFuture getALPN() { - assert connected(); return getConnectionFlow().getALPN(); } diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLConnection.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLConnection.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLConnection.java Thu Aug 09 11:23:12 2018 +0100 @@ -28,6 +28,8 @@ import java.net.InetSocketAddress; import java.nio.channels.SocketChannel; import java.util.concurrent.CompletableFuture; +import java.util.function.Function; +import jdk.internal.net.http.common.MinimalFuture; import jdk.internal.net.http.common.SSLTube; import jdk.internal.net.http.common.Utils; @@ -49,14 +51,9 @@ } @Override - PlainHttpConnection plainConnection() { - return plainConnection; - } - - @Override - public CompletableFuture connectAsync() { + public CompletableFuture connectAsync(Exchange> exchange) { return plainConnection - .connectAsync() + .connectAsync(exchange) .thenApply( unused -> { // create the SSLTube wrapping the SocketTube, with the given engine flow = new SSLTube(engine, @@ -67,6 +64,21 @@ } @Override + public CompletableFuture finishConnect() { + // The actual ALPN value, which may be the empty string, is not + // interesting at this point, only that the handshake has completed. + return getALPN() + .handle((String unused, Throwable ex) -> { + if (ex == null) { + return plainConnection.finishConnect(); + } else { + plainConnection.close(); + return MinimalFuture. failedFuture(ex); + } }) + .thenCompose(Function.identity()); + } + + @Override boolean connected() { return plainConnection.connected(); } diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLTunnelConnection.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLTunnelConnection.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLTunnelConnection.java Thu Aug 09 11:23:12 2018 +0100 @@ -29,6 +29,8 @@ import java.nio.channels.SocketChannel; import java.util.concurrent.CompletableFuture; import java.net.http.HttpHeaders; +import java.util.function.Function; +import jdk.internal.net.http.common.MinimalFuture; import jdk.internal.net.http.common.SSLTube; import jdk.internal.net.http.common.Utils; @@ -53,13 +55,13 @@ } @Override - public CompletableFuture connectAsync() { + public CompletableFuture connectAsync(Exchange> exchange) { if (debug.on()) debug.log("Connecting plain tunnel connection"); // This will connect the PlainHttpConnection flow, so that // its HttpSubscriber and HttpPublisher are subscribed to the // SocketTube return plainConnection - .connectAsync() + .connectAsync(exchange) .thenApply( unused -> { if (debug.on()) debug.log("creating SSLTube"); // create the SSLTube wrapping the SocketTube, with the given engine @@ -71,6 +73,21 @@ } @Override + public CompletableFuture finishConnect() { + // The actual ALPN value, which may be the empty string, is not + // interesting at this point, only that the handshake has completed. + return getALPN() + .handle((String unused, Throwable ex) -> { + if (ex == null) { + return plainConnection.finishConnect(); + } else { + plainConnection.close(); + return MinimalFuture. failedFuture(ex); + } }) + .thenCompose(Function.identity()); + } + + @Override boolean isTunnel() { return true; } @Override @@ -87,11 +104,6 @@ } @Override - PlainTunnelingConnection plainConnection() { - return plainConnection; - } - - @Override ConnectionPool.CacheKey cacheKey() { return ConnectionPool.cacheKey(address, plainConnection.proxyAddr); } diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java Thu Aug 09 11:23:12 2018 +0100 @@ -83,6 +83,10 @@ final PushGroup pushGroup; final String dbgTag; + // Keeps track of the underlying connection when establishing an HTTP/2 + // exchange so that it can be aborted/timed out mid setup. + final ConnectionAborter connectionAborter = new ConnectionAborter(); + Exchange(HttpRequestImpl request, MultiExchange multi) { this.request = request; this.upgrading = false; @@ -125,6 +129,27 @@ return client; } + // Keeps track of the underlying connection when establishing an HTTP/2 + // exchange so that it can be aborted/timed out mid setup. + static final class ConnectionAborter { + private volatile HttpConnection connection; + + void connection(HttpConnection connection) { + this.connection = connection; + } + + void closeConnection() { + HttpConnection connection = this.connection; + this.connection = null; + if (connection != null) { + try { + connection.close(); + } catch (Throwable t) { + // ignore + } + } + } + } public CompletableFuture readBodyAsync(HttpResponse.BodyHandler handler) { // If we received a 407 while establishing the exchange @@ -179,6 +204,7 @@ } public void cancel(IOException cause) { + if (debug.on()) debug.log("cancel exchImpl: %s, with \"%s\"", exchImpl, cause); // If the impl is non null, propagate the exception right away. // Otherwise record it so that it can be propagated once the // exchange impl has been established. @@ -190,6 +216,11 @@ } else { // no impl yet. record the exception failed = cause; + + // abort/close the connection if setting up the exchange. This can + // be important when setting up HTTP/2 + connectionAborter.closeConnection(); + // now call checkCancelled to recheck the impl. // if the failed state is set and the impl is not null, reset // the failed state and propagate the exception to the impl. diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java Thu Aug 09 11:23:12 2018 +0100 @@ -85,7 +85,7 @@ } else { Http2ClientImpl c2 = exchange.client().client2(); // #### improve HttpRequestImpl request = exchange.request(); - CompletableFuture c2f = c2.getConnectionFor(request); + CompletableFuture c2f = c2.getConnectionFor(request, exchange); if (debug.on()) debug.log("get: Trying to get HTTP/2 connection"); return c2f.handle((h2c, t) -> createExchangeImpl(h2c, t, exchange, connection)) diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java Thu Aug 09 11:23:12 2018 +0100 @@ -233,7 +233,8 @@ CompletableFuture connectCF; if (!connection.connected()) { if (debug.on()) debug.log("initiating connect async"); - connectCF = connection.connectAsync(); + connectCF = connection.connectAsync(exchange) + .thenCompose(unused -> connection.finishConnect()); Throwable cancelled; synchronized (lock) { if ((cancelled = failed) == null) { diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java Thu Aug 09 11:23:12 2018 +0100 @@ -90,7 +90,8 @@ * 3. completes normally with null: no connection in cache for h2c or h2 failed previously * 4. completes normally with connection: h2 or h2c connection in cache. Use it. */ - CompletableFuture getConnectionFor(HttpRequestImpl req) { + CompletableFuture getConnectionFor(HttpRequestImpl req, + Exchange> exchange) { URI uri = req.uri(); InetSocketAddress proxy = req.proxy(); String key = Http2Connection.keyFor(uri, proxy); @@ -123,7 +124,7 @@ } } return Http2Connection - .createAsync(req, this) + .createAsync(req, this, exchange) .whenComplete((conn, t) -> { synchronized (Http2ClientImpl.this) { if (conn != null) { diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java Thu Aug 09 11:23:12 2018 +0100 @@ -353,7 +353,8 @@ // Requires TLS handshake. So, is really async static CompletableFuture createAsync(HttpRequestImpl request, - Http2ClientImpl h2client) { + Http2ClientImpl h2client, + Exchange> exchange) { assert request.secure(); AbstractAsyncSSLConnection connection = (AbstractAsyncSSLConnection) HttpConnection.getConnection(request.getAddress(), @@ -361,7 +362,12 @@ request, HttpClient.Version.HTTP_2); - return connection.connectAsync() + // Expose the underlying connection to the exchange's aborter so it can + // be closed if a timeout occurs. + exchange.connectionAborter.connection(connection); + + return connection.connectAsync(exchange) + .thenCompose(unused -> connection.finishConnect()) .thenCompose(unused -> checkSSLConfig(connection)) .thenCompose(notused-> { CompletableFuture cf = new MinimalFuture<>(); diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/HttpClientBuilderImpl.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientBuilderImpl.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientBuilderImpl.java Thu Aug 09 11:23:12 2018 +0100 @@ -28,6 +28,7 @@ import java.net.Authenticator; import java.net.CookieHandler; import java.net.ProxySelector; +import java.time.Duration; import java.util.concurrent.Executor; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLParameters; @@ -38,6 +39,7 @@ public class HttpClientBuilderImpl implements HttpClient.Builder { CookieHandler cookieHandler; + Duration connectTimeout; HttpClient.Redirect followRedirects; ProxySelector proxy; Authenticator authenticator; @@ -55,6 +57,14 @@ return this; } + @Override + public HttpClientBuilderImpl connectTimeout(Duration duration) { + requireNonNull(duration); + if (duration.isNegative() || Duration.ZERO.equals(duration)) + throw new IllegalArgumentException("Invalid duration: " + duration); + this.connectTimeout = duration; + return this; + } @Override public HttpClientBuilderImpl sslContext(SSLContext sslContext) { diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/HttpClientFacade.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientFacade.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientFacade.java Thu Aug 09 11:23:12 2018 +0100 @@ -30,6 +30,7 @@ import java.net.Authenticator; import java.net.CookieHandler; import java.net.ProxySelector; +import java.time.Duration; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; @@ -70,6 +71,11 @@ } @Override + public Optional connectTimeout() { + return impl.connectTimeout(); + } + + @Override public Redirect followRedirects() { return impl.followRedirects(); } diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java Thu Aug 09 11:23:12 2018 +0100 @@ -35,6 +35,7 @@ import java.net.ConnectException; import java.net.CookieHandler; import java.net.ProxySelector; +import java.net.http.HttpConnectTimeoutException; import java.net.http.HttpTimeoutException; import java.nio.ByteBuffer; import java.nio.channels.CancelledKeyException; @@ -47,6 +48,7 @@ import java.security.AccessController; import java.security.NoSuchAlgorithmException; import java.security.PrivilegedAction; +import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; @@ -154,6 +156,7 @@ } private final CookieHandler cookieHandler; + private final Duration connectTimeout; private final Redirect followRedirects; private final Optional userProxySelector; private final ProxySelector proxySelector; @@ -278,6 +281,7 @@ facadeRef = new WeakReference<>(facadeFactory.createFacade(this)); client2 = new Http2ClientImpl(this); cookieHandler = builder.cookieHandler; + connectTimeout = builder.connectTimeout; followRedirects = builder.followRedirects == null ? Redirect.NEVER : builder.followRedirects; this.userProxySelector = Optional.ofNullable(builder.proxy); @@ -547,6 +551,10 @@ throw new IllegalArgumentException(msg, throwable); } else if (throwable instanceof SecurityException) { throw new SecurityException(msg, throwable); + } else if (throwable instanceof HttpConnectTimeoutException) { + HttpConnectTimeoutException hcte = new HttpConnectTimeoutException(msg); + hcte.initCause(throwable); + throw hcte; } else if (throwable instanceof HttpTimeoutException) { throw new HttpTimeoutException(msg); } else if (throwable instanceof ConnectException) { @@ -1124,6 +1132,11 @@ } @Override + public Optional connectTimeout() { + return Optional.ofNullable(connectTimeout); + } + + @Override public Optional proxy() { return this.userProxySelector; } diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java Thu Aug 09 11:23:12 2018 +0100 @@ -108,9 +108,20 @@ return client; } - //public abstract void connect() throws IOException, InterruptedException; + /** + * Initiates the connect phase. + * + * Returns a CompletableFuture that completes when the underlying + * TCP connection has been established or an error occurs. + */ + public abstract CompletableFuture connectAsync(Exchange> exchange); - public abstract CompletableFuture connectAsync(); + /** + * Finishes the connection phase. + * + * Returns a CompletableFuture that completes when any additional, + * type specific, setup has been done. Must be called after connectAsync. */ + public abstract CompletableFuture finishConnect(); /** Tells whether, or not, this connection is connected to its destination. */ abstract boolean connected(); diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java Thu Aug 09 11:23:12 2018 +0100 @@ -27,7 +27,7 @@ import java.io.IOException; import java.net.ConnectException; -import java.time.Duration; +import java.net.http.HttpConnectTimeoutException; import java.util.Iterator; import java.util.LinkedList; import java.security.AccessControlContext; @@ -88,7 +88,7 @@ ); private final LinkedList filters; - TimedEvent timedEvent; + ResponseTimerEvent responseTimerEvent; volatile boolean cancelled; final PushGroup pushGroup; @@ -134,7 +134,7 @@ this.exchange = new Exchange<>(request, this); } - private synchronized Exchange getExchange() { + synchronized Exchange getExchange() { return exchange; } @@ -157,8 +157,8 @@ } private void cancelTimer() { - if (timedEvent != null) { - client.cancelTimer(timedEvent); + if (responseTimerEvent != null) { + client.cancelTimer(responseTimerEvent); } } @@ -220,8 +220,8 @@ cf = failedFuture(new IOException("Too many retries", retryCause)); } else { if (currentreq.timeout().isPresent()) { - timedEvent = new TimedEvent(currentreq.timeout().get()); - client.registerTimer(timedEvent); + responseTimerEvent = ResponseTimerEvent.of(this); + client.registerTimer(responseTimerEvent); } try { // 1. apply request filters @@ -344,7 +344,9 @@ } } if (cancelled && t instanceof IOException) { - t = new HttpTimeoutException("request timed out"); + if (!(t instanceof HttpTimeoutException)) { + t = toTimeoutException((IOException)t); + } } else if (retryOnFailure(t)) { Throwable cause = retryCause(t); @@ -378,17 +380,24 @@ return failedFuture(t); } - class TimedEvent extends TimeoutEvent { - TimedEvent(Duration duration) { - super(duration); + private HttpTimeoutException toTimeoutException(IOException ioe) { + HttpTimeoutException t = null; + + // more specific, "request timed out", when connected + Exchange> exchange = getExchange(); + if (exchange != null) { + ExchangeImpl> exchangeImpl = exchange.exchImpl; + if (exchangeImpl != null) { + if (exchangeImpl.connection().connected()) { + t = new HttpTimeoutException("request timed out"); + t.initCause(ioe); + } + } } - @Override - public void handle() { - if (debug.on()) { - debug.log("Cancelling MultiExchange due to timeout for request %s", - request); - } - cancel(new HttpTimeoutException("request timed out")); + if (t == null) { + t = new HttpConnectTimeoutException("HTTP connect timed out"); + t.initCause(new ConnectException("HTTP connect timed out")); } + return t; } } diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java Thu Aug 09 11:23:12 2018 +0100 @@ -26,6 +26,7 @@ package jdk.internal.net.http; import java.io.IOException; +import java.net.ConnectException; import java.net.InetSocketAddress; import java.net.StandardSocketOptions; import java.nio.channels.SelectableChannel; @@ -34,6 +35,7 @@ import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; +import java.time.Duration; import java.util.concurrent.CompletableFuture; import jdk.internal.net.http.common.FlowTube; import jdk.internal.net.http.common.Log; @@ -53,9 +55,52 @@ private final PlainHttpPublisher writePublisher = new PlainHttpPublisher(reading); private volatile boolean connected; private boolean closed; + private volatile ConnectTimerEvent connectTimerEvent; // may be null // should be volatile to provide proper synchronization(visibility) action + /** + * Returns a ConnectTimerEvent iff there is a connect timeout duration, + * otherwise null. + */ + private ConnectTimerEvent newConnectTimer(Exchange> exchange, + CompletableFuture cf) { + Duration duration = client().connectTimeout().orElse(null); + if (duration != null) { + ConnectTimerEvent cte = new ConnectTimerEvent(duration, exchange, cf); + return cte; + } + return null; + } + + final class ConnectTimerEvent extends TimeoutEvent { + private final CompletableFuture cf; + private final Exchange> exchange; + + ConnectTimerEvent(Duration duration, + Exchange> exchange, + CompletableFuture cf) { + super(duration); + this.exchange = exchange; + this.cf = cf; + } + + @Override + public void handle() { + if (debug.on()) { + debug.log("HTTP connect timed out"); + } + ConnectException ce = new ConnectException("HTTP connect timed out"); + exchange.multi.cancel(ce); + client().theExecutor().execute(() -> cf.completeExceptionally(ce)); + } + + @Override + public String toString() { + return "ConnectTimerEvent, " + super.toString(); + } + } + final class ConnectEvent extends AsyncEvent { private final CompletableFuture cf; @@ -85,7 +130,6 @@ if (debug.on()) debug.log("ConnectEvent: connect finished: %s Local addr: %s", finished, chan.getLocalAddress()); - connected = true; // complete async since the event runs on the SelectorManager thread cf.completeAsync(() -> null, client().theExecutor()); } catch (Throwable e) { @@ -103,12 +147,20 @@ } @Override - public CompletableFuture connectAsync() { + public CompletableFuture connectAsync(Exchange> exchange) { CompletableFuture cf = new MinimalFuture<>(); try { assert !connected : "Already connected"; assert !chan.isBlocking() : "Unexpected blocking channel"; - boolean finished = false; + boolean finished; + + connectTimerEvent = newConnectTimer(exchange, cf); + if (connectTimerEvent != null) { + if (debug.on()) + debug.log("registering connect timer: " + connectTimerEvent); + client().registerTimer(connectTimerEvent); + } + PrivilegedExceptionAction pa = () -> chan.connect(Utils.resolveAddress(address)); try { @@ -118,7 +170,6 @@ } if (finished) { if (debug.on()) debug.log("connect finished without blocking"); - connected = true; cf.complete(null); } else { if (debug.on()) debug.log("registering connect event"); @@ -137,6 +188,16 @@ } @Override + public CompletableFuture finishConnect() { + assert connected == false; + if (debug.on()) debug.log("finishConnect, setting connected=true"); + connected = true; + if (connectTimerEvent != null) + client().cancelTimer(connectTimerEvent); + return MinimalFuture.completedFuture(null); + } + + @Override SocketChannel channel() { return chan; } @@ -210,6 +271,8 @@ Log.logTrace("Closing: " + toString()); if (debug.on()) debug.log("Closing channel: " + client().debugInterestOps(chan)); + if (connectTimerEvent != null) + client().cancelTimer(connectTimerEvent); chan.close(); tube.signalClosed(); } catch (IOException e) { diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/PlainTunnelingConnection.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/PlainTunnelingConnection.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/PlainTunnelingConnection.java Thu Aug 09 11:23:12 2018 +0100 @@ -26,11 +26,13 @@ package jdk.internal.net.http; import java.io.IOException; -import java.lang.System.Logger.Level; import java.net.InetSocketAddress; +import java.net.http.HttpTimeoutException; import java.nio.ByteBuffer; import java.nio.channels.SocketChannel; +import java.time.Duration; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.function.Function; import java.net.http.HttpHeaders; import jdk.internal.net.http.common.FlowTube; @@ -60,9 +62,10 @@ } @Override - public CompletableFuture connectAsync() { + public CompletableFuture connectAsync(Exchange> exchange) { if (debug.on()) debug.log("Connecting plain connection"); - return delegate.connectAsync() + return delegate.connectAsync(exchange) + .thenCompose(unused -> delegate.finishConnect()) .thenCompose((Void v) -> { if (debug.on()) debug.log("sending HTTP/1.1 CONNECT"); HttpClientImpl client = client(); @@ -70,7 +73,7 @@ HttpRequestImpl req = new HttpRequestImpl("CONNECT", address, proxyHeaders); MultiExchange mulEx = new MultiExchange<>(null, req, client, discarding(), null, null); - Exchange connectExchange = new Exchange<>(req, mulEx); + Exchange connectExchange = mulEx.getExchange(); return connectExchange .responseAsyncImpl(delegate) @@ -96,14 +99,36 @@ ByteBuffer b = ((Http1Exchange>)connectExchange.exchImpl).drainLeftOverBytes(); int remaining = b.remaining(); assert remaining == 0: "Unexpected remaining: " + remaining; - connected = true; cf.complete(null); } return cf; - }); + }) + .handle((result, ex) -> { + if (ex == null) { + return MinimalFuture.completedFuture(result); + } else { + if (debug.on()) + debug.log("tunnel failed with \"%s\"", ex.toString()); + Throwable t = ex; + if (t instanceof CompletionException) + t = t.getCause(); + if (t instanceof HttpTimeoutException) { + String msg = "proxy tunneling CONNECT request timed out"; + t = new HttpTimeoutException(msg); + t.initCause(ex); + } + return MinimalFuture. failedFuture(t); + } + }) + .thenCompose(Function.identity()); }); } + public CompletableFuture finishConnect() { + connected = true; + return MinimalFuture.completedFuture(null); + } + @Override boolean isTunnel() { return true; } diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/ResponseTimerEvent.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/ResponseTimerEvent.java Thu Aug 09 11:23:12 2018 +0100 @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2018, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.internal.net.http; + +import java.net.ConnectException; +import java.net.http.HttpConnectTimeoutException; +import java.net.http.HttpTimeoutException; +import jdk.internal.net.http.common.Logger; +import jdk.internal.net.http.common.Utils; + +public class ResponseTimerEvent extends TimeoutEvent { + private static final Logger debug = + Utils.getDebugLogger("ResponseTimerEvent"::toString, Utils.DEBUG); + + private final MultiExchange> multiExchange; + + static ResponseTimerEvent of(MultiExchange> exchange) { + return new ResponseTimerEvent(exchange); + } + + private ResponseTimerEvent(MultiExchange> multiExchange) { + super(multiExchange.exchange.request.timeout().get()); + this.multiExchange = multiExchange; + } + + @Override + public void handle() { + if (debug.on()) { + debug.log("Cancelling MultiExchange due to timeout for request %s", + multiExchange.exchange.request); + } + HttpTimeoutException t = null; + + // more specific, "request timed out", message when connected + Exchange> exchange = multiExchange.getExchange(); + if (exchange != null) { + ExchangeImpl> exchangeImpl = exchange.exchImpl; + if (exchangeImpl != null) { + if (exchangeImpl.connection().connected()) { + t = new HttpTimeoutException("request timed out"); + } + } + } + if (t == null) { + t = new HttpConnectTimeoutException("HTTP connect timed out"); + t.initCause(new ConnectException("HTTP connect timed out")); + } + multiExchange.cancel(t); + } + + @Override + public String toString() { + return "ResponseTimerEvent[" + super.toString() + "]"; + } +} diff -r a6fa2016cff1 -r 31d9e82b2e64 src/java.net.http/share/classes/jdk/internal/net/http/TimeoutEvent.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/TimeoutEvent.java Wed Aug 08 15:51:08 2018 -0700 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/TimeoutEvent.java Thu Aug 09 11:23:12 2018 +0100 @@ -43,9 +43,11 @@ // we use id in compareTo to make compareTo consistent with equals // see TimeoutEvent::compareTo below; private final long id = COUNTER.incrementAndGet(); + private final Duration duration; private final Instant deadline; TimeoutEvent(Duration duration) { + this.duration = duration; deadline = Instant.now().plus(duration); } @@ -75,6 +77,7 @@ @Override public String toString() { - return "TimeoutEvent[id=" + id + ", deadline=" + deadline + "]"; + return "TimeoutEvent[id=" + id + ", duration=" + duration + + ", deadline=" + deadline + "]"; } } diff -r a6fa2016cff1 -r 31d9e82b2e64 test/jdk/java/net/httpclient/AbstractConnectTimeout.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/net/httpclient/AbstractConnectTimeout.java Thu Aug 09 11:23:12 2018 +0100 @@ -0,0 +1,257 @@ +/* + * Copyright (c) 2018, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.net.ConnectException; +import java.net.InetSocketAddress; +import java.net.NoRouteToHostException; +import java.net.ProxySelector; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpClient.Version; +import java.net.http.HttpConnectTimeoutException; +import java.net.http.HttpRequest; +import java.net.http.HttpRequest.BodyPublishers; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.CompletionException; +import org.testng.annotations.DataProvider; +import static java.lang.System.out; +import static java.net.http.HttpClient.Builder.NO_PROXY; +import static java.net.http.HttpClient.Version.HTTP_1_1; +import static java.net.http.HttpClient.Version.HTTP_2; +import static java.time.Duration.*; +import static java.util.concurrent.TimeUnit.NANOSECONDS; +import static org.testng.Assert.fail; + +public abstract class AbstractConnectTimeout { + + static final Duration NO_DURATION = null; + + static List > TIMEOUTS = List.of( + // connectTimeout HttpRequest timeout + Arrays.asList( NO_DURATION, ofSeconds(1) ), + Arrays.asList( NO_DURATION, ofMillis(100) ), + Arrays.asList( NO_DURATION, ofNanos(99) ), + Arrays.asList( NO_DURATION, ofNanos(1) ), + + Arrays.asList( ofSeconds(1), NO_DURATION ), + Arrays.asList( ofMillis(100), NO_DURATION ), + Arrays.asList( ofNanos(99), NO_DURATION ), + Arrays.asList( ofNanos(1), NO_DURATION ), + + Arrays.asList( ofSeconds(1), ofMinutes(1) ), + Arrays.asList( ofMillis(100), ofMinutes(1) ), + Arrays.asList( ofNanos(99), ofMinutes(1) ), + Arrays.asList( ofNanos(1), ofMinutes(1) ) + ); + + static final List
METHODS = List.of("GET", "POST"); + static final List VERSIONS = List.of(HTTP_2, HTTP_1_1); + static final List SCHEMES = List.of("https", "http"); + + @DataProvider(name = "variants") + public Object[][] variants() { + List