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
This commit is contained in:
Daniel Fuchs 2020-12-10 10:09:29 +00:00
parent d93293f31b
commit 4a839e95de
6 changed files with 164 additions and 48 deletions

@ -471,7 +471,7 @@ class MultiExchange<T> implements Cancelable {
/** 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) {

@ -36,7 +36,10 @@ import java.security.AccessController;
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 @@ class PlainHttpConnection extends HttpConnection {
private volatile boolean connected;
private boolean closed;
private volatile ConnectTimerEvent connectTimerEvent; // may be null
private volatile int unsuccessfulAttempts;
// 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 }
// 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<Void> cf) {
CompletableFuture<?> cf) {
Duration duration = exchange.remainingConnectTimeout().orElse(null);
if (duration != null) {
ConnectTimerEvent cte = new ConnectTimerEvent(duration, exchange, cf);
@ -74,12 +82,12 @@ class PlainHttpConnection extends HttpConnection {
}
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 @@ class PlainHttpConnection extends HttpConnection {
}
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 @@ class PlainHttpConnection extends HttpConnection {
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 @@ class PlainHttpConnection extends HttpConnection {
@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 @@ class PlainHttpConnection extends HttpConnection {
}
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 @@ class PlainHttpConnection extends HttpConnection {
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

@ -48,6 +48,7 @@ import java.util.Map;
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 @@ public interface HttpServerAdapters {
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 @@ public interface HttpServerAdapters {
@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) {

@ -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.InetAddress;
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.ArrayList;
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 @@ import static org.testng.Assert.assertTrue;
* @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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
}
}
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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
@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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
}
}
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 @@ public class LineBodyHandlerTest implements HttpServerAdapters {
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";

@ -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.InetAddress;
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 @@ public class ManyRequests {
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 @@ public class ManyRequests {
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 @@ public class ManyRequests {
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);
}
}

@ -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