8256459: java/net/httpclient/ManyRequests.java and java/net/httpclient/LineBodyHandlerTest.java fail infrequently with java.net.ConnectException: Connection timed out: no further information
Reviewed-by: chegar
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java
index c1f0319..afa7113 100644
--- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java
@@ -471,7 +471,7 @@
/** True if ALL ( even non-idempotent ) requests can be automatic retried. */
private static final boolean RETRY_ALWAYS = retryPostValue();
/** True if ConnectException should cause a retry. Enabled by default */
- private static final boolean RETRY_CONNECT = !disableRetryConnect();
+ static final boolean RETRY_CONNECT = !disableRetryConnect();
/** Returns true is given request has an idempotent method. */
private static boolean isIdempotentRequest(HttpRequest request) {
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java b/src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java
index 7a550c5..db90759 100644
--- a/src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java
@@ -36,7 +36,10 @@
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.time.Duration;
+import java.time.Instant;
import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+
import jdk.internal.net.http.common.FlowTube;
import jdk.internal.net.http.common.Log;
import jdk.internal.net.http.common.MinimalFuture;
@@ -56,15 +59,20 @@
private volatile boolean connected;
private boolean closed;
private volatile ConnectTimerEvent connectTimerEvent; // may be null
+ private volatile int unsuccessfulAttempts;
- // should be volatile to provide proper synchronization(visibility) action
+ // Indicates whether a connection attempt has succeeded or should be retried.
+ // If the attempt failed, and shouldn't be retried, there will be an exception
+ // instead.
+ private enum ConnectState { SUCCESS, RETRY }
+
/**
* Returns a ConnectTimerEvent iff there is a connect timeout duration,
* otherwise null.
*/
private ConnectTimerEvent newConnectTimer(Exchange<?> exchange,
- CompletableFuture<Void> cf) {
+ CompletableFuture<?> cf) {
Duration duration = exchange.remainingConnectTimeout().orElse(null);
if (duration != null) {
ConnectTimerEvent cte = new ConnectTimerEvent(duration, exchange, cf);
@@ -74,12 +82,12 @@
}
final class ConnectTimerEvent extends TimeoutEvent {
- private final CompletableFuture<Void> cf;
+ private final CompletableFuture<?> cf;
private final Exchange<?> exchange;
ConnectTimerEvent(Duration duration,
Exchange<?> exchange,
- CompletableFuture<Void> cf) {
+ CompletableFuture<?> cf) {
super(duration);
this.exchange = exchange;
this.cf = cf;
@@ -102,10 +110,10 @@
}
final class ConnectEvent extends AsyncEvent {
- private final CompletableFuture<Void> cf;
+ private final CompletableFuture<ConnectState> cf;
private final Exchange<?> exchange;
- ConnectEvent(CompletableFuture<Void> cf, Exchange<?> exchange) {
+ ConnectEvent(CompletableFuture<ConnectState> cf, Exchange<?> exchange) {
this.cf = cf;
this.exchange = exchange;
}
@@ -133,8 +141,13 @@
finished, exchange.multi.requestCancelled(), chan.getLocalAddress());
assert finished || exchange.multi.requestCancelled() : "Expected channel to be connected";
// complete async since the event runs on the SelectorManager thread
- cf.completeAsync(() -> null, client().theExecutor());
+ cf.completeAsync(() -> ConnectState.SUCCESS, client().theExecutor());
} catch (Throwable e) {
+ if (canRetryConnect(e)) {
+ unsuccessfulAttempts++;
+ cf.completeAsync(() -> ConnectState.RETRY, client().theExecutor());
+ return;
+ }
Throwable t = Utils.toConnectException(e);
client().theExecutor().execute( () -> cf.completeExceptionally(t));
close();
@@ -150,17 +163,19 @@
@Override
public CompletableFuture<Void> connectAsync(Exchange<?> exchange) {
- CompletableFuture<Void> cf = new MinimalFuture<>();
+ CompletableFuture<ConnectState> cf = new MinimalFuture<>();
try {
assert !connected : "Already connected";
assert !chan.isBlocking() : "Unexpected blocking channel";
boolean finished;
- connectTimerEvent = newConnectTimer(exchange, cf);
- if (connectTimerEvent != null) {
- if (debug.on())
- debug.log("registering connect timer: " + connectTimerEvent);
- client().registerTimer(connectTimerEvent);
+ if (connectTimerEvent == null) {
+ connectTimerEvent = newConnectTimer(exchange, cf);
+ if (connectTimerEvent != null) {
+ if (debug.on())
+ debug.log("registering connect timer: " + connectTimerEvent);
+ client().registerTimer(connectTimerEvent);
+ }
}
PrivilegedExceptionAction<Boolean> pa =
@@ -172,7 +187,7 @@
}
if (finished) {
if (debug.on()) debug.log("connect finished without blocking");
- cf.complete(null);
+ cf.complete(ConnectState.SUCCESS);
} else {
if (debug.on()) debug.log("registering connect event");
client().registerEvent(new ConnectEvent(cf, exchange));
@@ -187,7 +202,44 @@
debug.log("Failed to close channel after unsuccessful connect");
}
}
- return cf;
+ return cf.handle((r,t) -> checkRetryConnect(r, t,exchange))
+ .thenCompose(Function.identity());
+ }
+
+ /**
+ * On some platforms, a ConnectEvent may be raised and a ConnectionException
+ * may occur with the message "Connection timed out: no further information"
+ * before our actual connection timeout has expired. In this case, this
+ * method will be called with a {@code connect} state of {@code ConnectState.RETRY)
+ * and we will retry once again.
+ * @param connect indicates whether the connection was successful or should be retried
+ * @param failed the failure if the connection failed
+ * @param exchange the exchange
+ * @return a completable future that will take care of retrying the connection if needed.
+ */
+ private CompletableFuture<Void> checkRetryConnect(ConnectState connect, Throwable failed, Exchange<?> exchange) {
+ // first check if the connection failed
+ if (failed != null) return MinimalFuture.failedFuture(failed);
+ // then check if the connection should be retried
+ if (connect == ConnectState.RETRY) {
+ int attempts = unsuccessfulAttempts;
+ assert attempts <= 1;
+ if (debug.on())
+ debug.log("Retrying connect after %d attempts", attempts);
+ return connectAsync(exchange);
+ }
+ // Otherwise, the connection was successful;
+ assert connect == ConnectState.SUCCESS;
+ return MinimalFuture.completedFuture(null);
+ }
+
+ private boolean canRetryConnect(Throwable e) {
+ if (!MultiExchange.RETRY_CONNECT) return false;
+ if (!(e instanceof ConnectException)) return false;
+ if (unsuccessfulAttempts > 0) return false;
+ ConnectTimerEvent timer = connectTimerEvent;
+ if (timer == null) return true;
+ return timer.deadline().isAfter(Instant.now());
}
@Override
diff --git a/test/jdk/java/net/httpclient/HttpServerAdapters.java b/test/jdk/java/net/httpclient/HttpServerAdapters.java
index 07a836b..b52d764 100644
--- a/test/jdk/java/net/httpclient/HttpServerAdapters.java
+++ b/test/jdk/java/net/httpclient/HttpServerAdapters.java
@@ -48,6 +48,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Stream;
@@ -505,13 +506,23 @@
return new Http1TestServer(server);
}
+ public static HttpTestServer of(HttpServer server, ExecutorService executor) {
+ return new Http1TestServer(server, executor);
+ }
+
public static HttpTestServer of(Http2TestServer server) {
return new Http2TestServerImpl(server);
}
private static class Http1TestServer extends HttpTestServer {
private final HttpServer impl;
+ private final ExecutorService executor;
Http1TestServer(HttpServer server) {
+ this(server, null);
+ }
+ Http1TestServer(HttpServer server, ExecutorService executor) {
+ if (executor != null) server.setExecutor(executor);
+ this.executor = executor;
this.impl = server;
}
@Override
@@ -522,7 +533,13 @@
@Override
public void stop() {
System.out.println("Http1TestServer: stop");
- impl.stop(0);
+ try {
+ impl.stop(0);
+ } finally {
+ if (executor != null) {
+ executor.shutdownNow();
+ }
+ }
}
@Override
public HttpTestContext addHandler(HttpTestHandler handler, String path) {
diff --git a/test/jdk/java/net/httpclient/LineBodyHandlerTest.java b/test/jdk/java/net/httpclient/LineBodyHandlerTest.java
index 039bc9c..7b29941 100644
--- a/test/jdk/java/net/httpclient/LineBodyHandlerTest.java
+++ b/test/jdk/java/net/httpclient/LineBodyHandlerTest.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, 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
@@ -32,6 +32,7 @@
import java.net.InetSocketAddress;
import java.net.URI;
import java.net.http.HttpClient;
+import java.net.http.HttpClient.Builder;
import java.net.http.HttpRequest;
import java.net.http.HttpRequest.BodyPublishers;
import java.net.http.HttpResponse;
@@ -43,7 +44,11 @@
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
import java.util.concurrent.Flow;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
@@ -71,6 +76,7 @@
* @summary Basic tests for line adapter subscribers as created by
* the BodyHandlers returned by BodyHandler::fromLineSubscriber
* and BodyHandler::asLines
+ * @bug 8256459
* @modules java.base/sun.net.www.http
* java.net.http/jdk.internal.net.http.common
* java.net.http/jdk.internal.net.http.frame
@@ -182,11 +188,17 @@
}
}
+ HttpClient newClient() {
+ return HttpClient.newBuilder()
+ .sslContext(sslContext)
+ .proxy(Builder.NO_PROXY)
+ .build();
+ }
+
@Test(dataProvider = "uris")
void testStringWithFinisher(String url) {
String body = "May the luck of the Irish be with you!";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext)
- .build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -207,8 +219,7 @@
@Test(dataProvider = "uris")
void testAsStream(String url) {
String body = "May the luck of the Irish be with you!";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext)
- .build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -230,7 +241,8 @@
@Test(dataProvider = "uris")
void testStringWithFinisher2(String url) {
String body = "May the luck\r\n\r\n of the Irish be with you!";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
+
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -251,7 +263,7 @@
@Test(dataProvider = "uris")
void testAsStreamWithCRLF(String url) {
String body = "May the luck\r\n\r\n of the Irish be with you!";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -275,7 +287,7 @@
@Test(dataProvider = "uris")
void testStringWithFinisherBlocking(String url) throws Exception {
String body = "May the luck of the Irish be with you!";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body)).build();
@@ -292,7 +304,7 @@
@Test(dataProvider = "uris")
void testStringWithoutFinisherBlocking(String url) throws Exception {
String body = "May the luck of the Irish be with you!";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body)).build();
@@ -311,7 +323,7 @@
@Test(dataProvider = "uris")
void testAsStreamWithMixedCRLF(String url) {
String body = "May\r\n the wind\r\n always be\rat your back.\r\r";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -338,7 +350,7 @@
@Test(dataProvider = "uris")
void testAsStreamWithMixedCRLF_UTF8(String url) {
String body = "May\r\n the wind\r\n always be\rat your back.\r\r";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.header("Content-type", "text/text; charset=UTF-8")
.POST(BodyPublishers.ofString(body, UTF_8)).build();
@@ -364,7 +376,7 @@
@Test(dataProvider = "uris")
void testAsStreamWithMixedCRLF_UTF16(String url) {
String body = "May\r\n the wind\r\n always be\rat your back.\r\r";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.header("Content-type", "text/text; charset=UTF-16")
.POST(BodyPublishers.ofString(body, UTF_16)).build();
@@ -391,7 +403,7 @@
@Test(dataProvider = "uris")
void testObjectWithFinisher(String url) {
String body = "May\r\n the wind\r\n always be\rat your back.";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -416,7 +428,7 @@
@Test(dataProvider = "uris")
void testObjectWithFinisher_UTF16(String url) {
String body = "May\r\n the wind\r\n always be\rat your back.\r\r";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.header("Content-type", "text/text; charset=UTF-16")
.POST(BodyPublishers.ofString(body, UTF_16)).build();
@@ -442,8 +454,7 @@
@Test(dataProvider = "uris")
void testObjectWithoutFinisher(String url) {
String body = "May\r\n the wind\r\n always be\rat your back.";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext)
- .build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -469,8 +480,7 @@
@Test(dataProvider = "uris")
void testObjectWithFinisherBlocking(String url) throws Exception {
String body = "May\r\n the wind\r\n always be\nat your back.";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext)
- .build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -494,8 +504,7 @@
@Test(dataProvider = "uris")
void testObjectWithoutFinisherBlocking(String url) throws Exception {
String body = "May\r\n the wind\r\n always be\nat your back.";
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext)
- .build();
+ HttpClient client = newClient();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(body))
.build();
@@ -529,8 +538,7 @@
@Test(dataProvider = "uris")
void testBigTextFromLineSubscriber(String url) {
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext)
- .build();
+ HttpClient client = newClient();
String bigtext = bigtext();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(bigtext))
@@ -551,8 +559,7 @@
@Test(dataProvider = "uris")
void testBigTextAsStream(String url) {
- HttpClient client = HttpClient.newBuilder().sslContext(sslContext)
- .build();
+ HttpClient client = newClient();
String bigtext = bigtext();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
.POST(BodyPublishers.ofString(bigtext))
@@ -640,6 +647,19 @@
}
}
+ private static ExecutorService executorFor(String serverThreadName) {
+ ThreadFactory factory = new ThreadFactory() {
+ final AtomicInteger counter = new AtomicInteger();
+ @Override
+ public Thread newThread(Runnable r) {
+ Thread thread = new Thread(r);
+ thread.setName(serverThreadName + "#" + counter.incrementAndGet());
+ return thread;
+ }
+ };
+ return Executors.newCachedThreadPool(factory);
+ }
+
@BeforeTest
public void setup() throws Exception {
sslContext = new SimpleSSLContext().get();
@@ -647,13 +667,15 @@
throw new AssertionError("Unexpected null sslContext");
InetSocketAddress sa = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
- httpTestServer = HttpTestServer.of(HttpServer.create(sa, 0));
+ httpTestServer = HttpTestServer.of(HttpServer.create(sa, 0),
+ executorFor("HTTP/1.1 Server Thread"));
httpTestServer.addHandler(new HttpTestEchoHandler(), "/http1/echo");
httpURI = "http://" + httpTestServer.serverAuthority() + "/http1/echo";
HttpsServer httpsServer = HttpsServer.create(sa, 0);
httpsServer.setHttpsConfigurator(new HttpsConfigurator(sslContext));
- httpsTestServer = HttpTestServer.of(httpsServer);
+ httpsTestServer = HttpTestServer.of(httpsServer,
+ executorFor("HTTPS/1.1 Server Thread"));
httpsTestServer.addHandler(new HttpTestEchoHandler(),"/https1/echo");
httpsURI = "https://" + httpsTestServer.serverAuthority() + "/https1/echo";
diff --git a/test/jdk/java/net/httpclient/ManyRequests.java b/test/jdk/java/net/httpclient/ManyRequests.java
index 97435d7..f79d565 100644
--- a/test/jdk/java/net/httpclient/ManyRequests.java
+++ b/test/jdk/java/net/httpclient/ManyRequests.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 8087112 8180044
+ * @bug 8087112 8180044 8256459
* @modules java.net.http
* java.logging
* jdk.httpserver
@@ -51,14 +51,20 @@
import java.net.InetSocketAddress;
import java.net.URI;
import java.net.http.HttpClient;
+import java.net.http.HttpClient.Builder;
import java.net.http.HttpRequest;
import java.net.http.HttpRequest.BodyPublishers;
import java.net.http.HttpResponse.BodyHandlers;
+import java.time.Duration;
import java.util.Arrays;
import java.util.Formatter;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.Random;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.Logger;
import java.util.logging.Level;
import java.util.concurrent.CompletableFuture;
@@ -81,16 +87,21 @@
InetSocketAddress addr = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
HttpsServer server = HttpsServer.create(addr, 0);
+ ExecutorService executor = executorFor("HTTPS/1.1 Server Thread");
server.setHttpsConfigurator(new Configurator(ctx));
+ server.setExecutor(executor);
HttpClient client = HttpClient.newBuilder()
+ .proxy(Builder.NO_PROXY)
.sslContext(ctx)
+ .connectTimeout(Duration.ofMillis(120_000)) // 2mins
.build();
try {
test(server, client);
System.out.println("OK");
} finally {
server.stop(0);
+ executor.shutdownNow();
}
}
@@ -102,7 +113,7 @@
static final boolean XFIXED = Boolean.getBoolean("test.XFixed");
static class TestEchoHandler extends EchoHandler {
- final Random rand = new Random();
+ final Random rand = jdk.test.lib.RandomFactory.getRandom();
@Override
public void handle(HttpExchange e) throws IOException {
System.out.println("Server: received " + e.getRequestURI());
@@ -279,4 +290,18 @@
params.setSSLParameters(getSSLContext().getSupportedSSLParameters());
}
}
+
+ private static ExecutorService executorFor(String serverThreadName) {
+ ThreadFactory factory = new ThreadFactory() {
+ final AtomicInteger counter = new AtomicInteger();
+ @Override
+ public Thread newThread(Runnable r) {
+ Thread thread = new Thread(r);
+ thread.setName(serverThreadName + "#" + counter.incrementAndGet());
+ return thread;
+ }
+ };
+ return Executors.newCachedThreadPool(factory);
+ }
+
}
diff --git a/test/jdk/java/net/httpclient/ManyRequests2.java b/test/jdk/java/net/httpclient/ManyRequests2.java
index f600b00..b0eee6e 100644
--- a/test/jdk/java/net/httpclient/ManyRequests2.java
+++ b/test/jdk/java/net/httpclient/ManyRequests2.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2020, 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
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 8087112 8180044
+ * @bug 8087112 8180044 8256459
* @modules java.net.http
* java.logging
* jdk.httpserver
