# HG changeset patch # User dfuchs # Date 1511792978 0 # Node ID 050803da27e530e1c40509ea495105397994b12f # Parent f68c4c641c48ea7a443e7d3f99e1479540f0f61c http-client-branch: review comments: TimeoutBasic.java is ignored diff -r f68c4c641c48 -r 050803da27e5 src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java Mon Nov 27 13:53:52 2017 +0000 +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java Mon Nov 27 14:29:38 2017 +0000 @@ -64,6 +64,7 @@ final HttpRequestImpl request; final HttpClientImpl client; volatile ExchangeImpl exchImpl; + volatile CompletableFuture> exchangeCF; // used to record possible cancellation raised before the exchImpl // has been established. private volatile IOException failed; @@ -190,10 +191,12 @@ private void checkCancelled() { ExchangeImpl impl = null; IOException cause = null; + CompletableFuture> cf = null; if (failed != null) { synchronized(this) { cause = failed; impl = exchImpl; + cf = exchangeCF; } } if (cause == null) return; @@ -212,6 +215,7 @@ (request.timeout().get().getSeconds() * 1000 + request.timeout().get().getNano() / 1000000) : -1, cause); + if (cf != null) cf.completeExceptionally(cause); } } @@ -228,14 +232,31 @@ // potential concurrent calls to cancel() or cancel(IOException) private CompletableFuture> establishExchange(HttpConnection connection) { + if (debug.isLoggable(Level.DEBUG)) { + debug.log(Level.DEBUG, + "establishing exchange for %s,%n\t proxy=%s", + request, + request.proxy()); + } // check if we have been cancelled first. Throwable t = getCancelCause(); checkCancelled(); if (t != null) { return MinimalFuture.failedFuture(t); } - CompletableFuture> cf = ExchangeImpl.get(this, connection); - return cf.thenCompose((eimpl) -> { + + CompletableFuture> cf, res; + cf = ExchangeImpl.get(this, connection); + // We should probably use a VarHandle to get/set exchangeCF + // instead - as we need CAS semantics. + synchronized (this) { exchangeCF = cf; }; + res = cf.whenComplete((r,x) -> { + synchronized(Exchange.this) { + if (exchangeCF == cf) exchangeCF = null; + } + }); + checkCancelled(); + return res.thenCompose((eimpl) -> { // recheck for cancelled, in case of race conditions exchImpl = eimpl; IOException tt = getCancelCause(); diff -r f68c4c641c48 -r 050803da27e5 test/jdk/java/net/httpclient/SplitResponse.java --- a/test/jdk/java/net/httpclient/SplitResponse.java Mon Nov 27 13:53:52 2017 +0000 +++ b/test/jdk/java/net/httpclient/SplitResponse.java Mon Nov 27 14:29:38 2017 +0000 @@ -24,8 +24,6 @@ import java.io.IOException; import java.net.URI; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorService; import javax.net.ssl.SSLContext; import javax.net.ServerSocketFactory; import javax.net.ssl.SSLServerSocketFactory; diff -r f68c4c641c48 -r 050803da27e5 test/jdk/java/net/httpclient/TimeoutBasic.java --- a/test/jdk/java/net/httpclient/TimeoutBasic.java Mon Nov 27 13:53:52 2017 +0000 +++ b/test/jdk/java/net/httpclient/TimeoutBasic.java Mon Nov 27 14:29:38 2017 +0000 @@ -28,66 +28,157 @@ import jdk.incubator.http.HttpRequest; import jdk.incubator.http.HttpResponse; import jdk.incubator.http.HttpTimeoutException; +import jdk.testlibrary.SimpleSSLContext; + +import javax.net.ServerSocketFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLServerSocketFactory; import java.time.Duration; +import java.util.Arrays; import java.util.List; -import java.util.concurrent.*; +import java.util.concurrent.CompletionException; +import java.util.function.Function; import static java.lang.System.out; import static jdk.incubator.http.HttpResponse.BodyHandler.discard; /** * @test + * @library /lib/testlibrary + * @build jdk.testlibrary.SimpleSSLContext * @summary Basic tests for response timeouts * @run main/othervm TimeoutBasic - * @ignore */ public class TimeoutBasic { - static List TIMEOUTS = List.of(/*Duration.ofSeconds(1), - Duration.ofMillis(100),*/ - Duration.ofNanos(99) - /* Duration.ofNanos(1)*/); + static List TIMEOUTS = List.of(Duration.ofSeconds(1), + Duration.ofMillis(100), + Duration.ofNanos(99), + Duration.ofNanos(1)); + + static final List> METHODS = + Arrays.asList(HttpRequest.Builder::GET, + TimeoutBasic::DELETE, + TimeoutBasic::PUT, + TimeoutBasic::POST, + null); + + static final List VERSIONS = + Arrays.asList(HttpClient.Version.HTTP_2, HttpClient.Version.HTTP_1_1, null); + + static final List SCHEMES = List.of("https", "http"); + + static { + try { + SSLContext.setDefault(new SimpleSSLContext().get()); + } catch (IOException x) { + throw new ExceptionInInitializerError(x); + } + } public static void main(String[] args) throws Exception { - HttpClient client = HttpClient.newHttpClient(); - try (ServerSocket ss = new ServerSocket(0, 20)) { - int port = ss.getLocalPort(); - URI uri = new URI("http://127.0.0.1:" + port + "/"); + for (Function m : METHODS) { + for (HttpClient.Version version : List.of(HttpClient.Version.HTTP_1_1)) { + for (HttpClient.Version reqVersion : VERSIONS) { + for (String scheme : SCHEMES) { + ServerSocketFactory ssf; + if (scheme.equalsIgnoreCase("https")) { + ssf = SSLServerSocketFactory.getDefault(); + } else { + ssf = ServerSocketFactory.getDefault(); + } + test(version, reqVersion, scheme, m, ssf); + } + } + } + } + } + + static HttpRequest.Builder DELETE(HttpRequest.Builder builder) { + HttpRequest.BodyPublisher noBody = HttpRequest.BodyPublisher.noBody(); + return builder.DELETE(noBody); + } + + static HttpRequest.Builder PUT(HttpRequest.Builder builder) { + HttpRequest.BodyPublisher noBody = HttpRequest.BodyPublisher.noBody(); + return builder.PUT(noBody); + } + + static HttpRequest.Builder POST(HttpRequest.Builder builder) { + HttpRequest.BodyPublisher noBody = HttpRequest.BodyPublisher.noBody(); + return builder.POST(noBody); + } -// out.println("--- TESTING Async"); -// for (Duration duration : TIMEOUTS) { -// out.println(" with duration of " + duration); -// HttpRequest request = HttpRequest.newBuilder(uri) -// .timeout(duration) -// .GET().build(); -// try { -// HttpResponse resp = client.sendAsync(request, discard(null)).join(); -// throw new RuntimeException("Unexpected response: " + resp.statusCode()); -// } catch (CompletionException e) { -// if (!(e.getCause() instanceof HttpTimeoutException)) { -// throw new RuntimeException("Unexpected exception: " + e.getCause()); -// } else { -// out.println("Caught expected timeout: " + e.getCause()); -// } -// } -// } + static HttpRequest newRequest(URI uri, + Duration duration, + HttpClient.Version reqVersion, + Function method) { + HttpRequest.Builder reqBuilder = HttpRequest.newBuilder(uri) + .timeout(duration); + if (method != null) reqBuilder = method.apply(reqBuilder); + if (reqVersion != null) reqBuilder = reqBuilder.version(reqVersion); + HttpRequest request = reqBuilder.build(); + if (duration.compareTo(Duration.ofSeconds(1)) >= 0) { + if (method == null || !request.method().equalsIgnoreCase("get")) { + out.println("Skipping " + duration + " for " + request.method()); + return null; + } + } + return request; + } + + public static void test(HttpClient.Version version, + HttpClient.Version reqVersion, + String scheme, + Function method, + ServerSocketFactory ssf) + throws Exception + { + HttpClient.Builder builder = HttpClient.newBuilder() + .proxy(HttpClient.Builder.NO_PROXY); + if (version != null) builder.version(version); + HttpClient client = builder.build(); + out.printf("%ntest(version=%s, reqVersion=%s, scheme=%s)%n", version, reqVersion, scheme); + try (ServerSocket ss = ssf.createServerSocket(0, 20)) { + int port = ss.getLocalPort(); + URI uri = new URI(scheme +"://127.0.0.1:" + port + "/"); + + out.println("--- TESTING Async"); + int count = 0; + for (Duration duration : TIMEOUTS) { + out.println(" with duration of " + duration); + HttpRequest request = newRequest(uri, duration, reqVersion, method); + if (request == null) continue; + count++; + try { + HttpResponse resp = client.sendAsync(request, discard(null)).join(); + throw new RuntimeException("Unexpected response: " + resp.statusCode()); + } catch (CompletionException e) { + if (!(e.getCause() instanceof HttpTimeoutException)) { + throw new RuntimeException("Unexpected exception: " + e.getCause()); + } else { + out.println("Caught expected timeout: " + e.getCause()); + } + } + } + assert count >= TIMEOUTS.size() -1; out.println("--- TESTING Sync"); + count = 0; for (Duration duration : TIMEOUTS) { out.println(" with duration of " + duration); - HttpRequest request = HttpRequest.newBuilder(uri) - .timeout(duration) - .GET() - .build(); + HttpRequest request = newRequest(uri, duration, reqVersion, method); + if (request == null) continue; + count++; try { client.send(request, discard(null)); } catch (HttpTimeoutException e) { out.println("Caught expected timeout: " + e); } } - } finally { - ((ExecutorService) client.executor()).shutdownNow(); + assert count >= TIMEOUTS.size() -1; + } } }