http-client-branch: review comments: TimeoutBasic.java is ignored http-client-branch
authordfuchs
Mon, 27 Nov 2017 14:29:38 +0000
branchhttp-client-branch
changeset 55891 050803da27e5
parent 55890 f68c4c641c48
child 55892 9f345a976249
http-client-branch: review comments: TimeoutBasic.java is ignored
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java
test/jdk/java/net/httpclient/SplitResponse.java
test/jdk/java/net/httpclient/TimeoutBasic.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<T> exchImpl;
+    volatile CompletableFuture<? extends ExchangeImpl<T>> 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<? extends ExchangeImpl<T>> 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<? extends ExchangeImpl<T>>
     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<? extends ExchangeImpl<T>> cf = ExchangeImpl.get(this, connection);
-        return cf.thenCompose((eimpl) -> {
+
+        CompletableFuture<? extends ExchangeImpl<T>> 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();
--- 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;
--- 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<Duration> TIMEOUTS = List.of(/*Duration.ofSeconds(1),
-                                             Duration.ofMillis(100),*/
-                                             Duration.ofNanos(99)
-                                            /* Duration.ofNanos(1)*/);
+    static List<Duration> TIMEOUTS = List.of(Duration.ofSeconds(1),
+                                             Duration.ofMillis(100),
+                                             Duration.ofNanos(99),
+                                             Duration.ofNanos(1));
+
+    static final List<Function<HttpRequest.Builder, HttpRequest.Builder>> METHODS =
+            Arrays.asList(HttpRequest.Builder::GET,
+                          TimeoutBasic::DELETE,
+                          TimeoutBasic::PUT,
+                          TimeoutBasic::POST,
+                          null);
+
+    static final List<HttpClient.Version> VERSIONS =
+            Arrays.asList(HttpClient.Version.HTTP_2, HttpClient.Version.HTTP_1_1, null);
+
+    static final List<String> 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<HttpRequest.Builder, HttpRequest.Builder> 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<HttpRequest.Builder, HttpRequest.Builder> 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<HttpRequest.Builder, HttpRequest.Builder> 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;
+
         }
     }
 }