changeset 17233:ea0146845b79

8180044: java/net/httpclient/ManyRequests.java failed due to timeout Summary: Fixes several race conditions observed while testing. Reviewed-by: michaelm, msheppar, prappo
author dfuchs
date Thu, 08 Jun 2017 12:41:07 +0100
parents 4287c7853433
children 3abaf7610609
files src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Stream.java test/com/sun/net/httpserver/FileServerHandler.java test/java/net/httpclient/ManyRequests.java test/java/net/httpclient/ManyRequests2.java
diffstat 6 files changed, 140 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java	Thu Jun 08 12:24:13 2017 +0100
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java	Thu Jun 08 12:41:07 2017 +0100
@@ -344,7 +344,13 @@
                                 c.configureBlocking(false);
                                 SelectionKey key = c.keyFor(selector);
                                 SelectorAttachment sa;
-                                if (key == null) {
+                                if (key == null || !key.isValid()) {
+                                    if (key != null) {
+                                        // key is canceled.
+                                        // invoke selectNow() to purge it
+                                        // before registering the new event.
+                                        selector.selectNow();
+                                    }
                                     sa = new SelectorAttachment(c, selector);
                                 } else {
                                     sa = (SelectorAttachment) key.attachment();
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java	Thu Jun 08 12:24:13 2017 +0100
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java	Thu Jun 08 12:41:07 2017 +0100
@@ -62,6 +62,7 @@
     private volatile Consumer<ByteBufferReference> asyncReceiver;
     private volatile Consumer<Throwable> errorReceiver;
     private volatile Supplier<ByteBufferReference> readBufferSupplier;
+    private boolean asyncReading;
 
     private final AsyncWriteQueue asyncOutputQ = new AsyncWriteQueue(this::asyncOutput);
 
@@ -70,6 +71,9 @@
     @Override
     public void startReading() {
         try {
+            synchronized(reading) {
+                asyncReading = true;
+            }
             client.registerEvent(new ReadEvent());
         } catch (IOException e) {
             shutdown();
@@ -78,6 +82,9 @@
 
     @Override
     public void stopAsyncReading() {
+        synchronized(reading) {
+            asyncReading = false;
+        }
         client.cancelRegistration(chan);
     }
 
@@ -279,7 +286,7 @@
     void asyncRead() {
         synchronized (reading) {
             try {
-                while (true) {
+                while (asyncReading) {
                     ByteBufferReference buf = readBufferSupplier.get();
                     int n = chan.read(buf.get());
                     if (n == -1) {
@@ -325,7 +332,7 @@
             return -1;
         }
         Utils.flipToMark(buf, mark);
-        String s = "Receive (" + n + " bytes) ";
+        // String s = "Receive (" + n + " bytes) ";
         //debugPrint(s, buf);
         return n;
     }
@@ -393,6 +400,10 @@
             shutdown();
         }
 
+        @Override
+        public String toString() {
+            return super.toString() + "/" + chan;
+        }
     }
 
     // used in blocking channels only
@@ -422,6 +433,11 @@
         public void abort() {
             close();
         }
+
+        @Override
+        public String toString() {
+            return super.toString() + "/" + chan;
+        }
     }
 
     @Override
@@ -447,7 +463,8 @@
     CompletableFuture<Void> whenReceivingResponse() {
         CompletableFuture<Void> cf = new MinimalFuture<>();
         try {
-            client.registerEvent(new ReceiveResponseEvent(cf));
+            ReceiveResponseEvent evt = new ReceiveResponseEvent(cf);
+            client.registerEvent(evt);
         } catch (IOException e) {
             cf.completeExceptionally(e);
         }
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Stream.java	Thu Jun 08 12:24:13 2017 +0100
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Stream.java	Thu Jun 08 12:41:07 2017 +0100
@@ -803,7 +803,9 @@
         completeResponseExceptionally(e);
         try {
             // will send a RST_STREAM frame
-            connection.resetStream(streamid, ResetFrame.CANCEL);
+            if (streamid != 0) {
+                connection.resetStream(streamid, ResetFrame.CANCEL);
+            }
         } catch (IOException ex) {
             Log.logError(ex);
         }
--- a/test/com/sun/net/httpserver/FileServerHandler.java	Thu Jun 08 12:24:13 2017 +0100
+++ b/test/com/sun/net/httpserver/FileServerHandler.java	Thu Jun 08 12:41:07 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2017, 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
@@ -214,8 +214,8 @@
             t.sendResponseHeaders(200, in.length);
             OutputStream os = t.getResponseBody();
             os.write(in);
-            os.close();
-            is.close();
+            close(os);
+            close(is);
         } else {
             OutputStream os = t.getResponseBody();
             byte[] buf = new byte[64 * 1024];
@@ -232,9 +232,15 @@
                 String s = Integer.toString(count);
                 os.write(s.getBytes());
             }
+            close(os);
+            close(is);
+        }
+    }
+
+    protected void close(OutputStream os) throws IOException {
             os.close();
+    }
+    protected void close(InputStream is) throws IOException {
             is.close();
         }
     }
-}
-
--- a/test/java/net/httpclient/ManyRequests.java	Thu Jun 08 12:24:13 2017 +0100
+++ b/test/java/net/httpclient/ManyRequests.java	Thu Jun 08 12:41:07 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
+ * @bug 8087112 8180044
  * @modules jdk.incubator.httpclient
  *          java.logging
  *          jdk.httpserver
@@ -32,13 +32,20 @@
  * @compile ../../../com/sun/net/httpserver/LogFilter.java
  * @compile ../../../com/sun/net/httpserver/FileServerHandler.java
  * @run main/othervm/timeout=40 -Djdk.httpclient.HttpClient.log=ssl ManyRequests
+ * @run main/othervm/timeout=40 -Dtest.insertDelay=true ManyRequests
+ * @run main/othervm/timeout=40 -Dtest.chunkSize=64 ManyRequests
+ * @run main/othervm/timeout=40 -Dtest.insertDelay=true -Dtest.chunkSize=64 ManyRequests
  * @summary Send a large number of requests asynchronously
  */
+ // * @run main/othervm/timeout=40 -Djdk.httpclient.HttpClient.log=ssl ManyRequests
 
 import com.sun.net.httpserver.HttpsConfigurator;
 import com.sun.net.httpserver.HttpsParameters;
 import com.sun.net.httpserver.HttpsServer;
+import com.sun.net.httpserver.HttpExchange;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import jdk.incubator.http.HttpClient;
 import jdk.incubator.http.HttpRequest;
 import java.net.InetSocketAddress;
@@ -65,7 +72,10 @@
         Logger logger = Logger.getLogger("com.sun.net.httpserver");
         logger.setLevel(Level.ALL);
         logger.info("TEST");
-
+        System.out.println("Sending " + REQUESTS
+                         + " requests; delay=" + INSERT_DELAY
+                         + ", chunks=" + CHUNK_SIZE
+                         + ", XFixed=" + XFIXED);
         SSLContext ctx = new SimpleSSLContext().get();
 
         InetSocketAddress addr = new InetSocketAddress(0);
@@ -86,11 +96,36 @@
 
     //static final int REQUESTS = 1000;
     static final int REQUESTS = 20;
+    static final boolean INSERT_DELAY = Boolean.getBoolean("test.insertDelay");
+    static final int CHUNK_SIZE = Math.max(0,
+           Integer.parseInt(System.getProperty("test.chunkSize", "0")));
+    static final boolean XFIXED = Boolean.getBoolean("test.XFixed");
+
+    static class TestEchoHandler extends EchoHandler {
+        final Random rand = new Random();
+        @Override
+        public void handle(HttpExchange e) throws IOException {
+            System.out.println("Server: received " + e.getRequestURI());
+            super.handle(e);
+        }
+        protected void close(OutputStream os) throws IOException {
+            if (INSERT_DELAY) {
+                try { Thread.sleep(rand.nextInt(200)); } catch (InterruptedException e) {}
+            }
+            super.close(os);
+        }
+        protected void close(InputStream is) throws IOException {
+            if (INSERT_DELAY) {
+                try { Thread.sleep(rand.nextInt(200)); } catch (InterruptedException e) {}
+            }
+            super.close(is);
+        }
+    }
 
     static void test(HttpsServer server, HttpClient client) throws Exception {
         int port = server.getAddress().getPort();
-        URI uri = new URI("https://127.0.0.1:" + port + "/foo/x");
-        server.createContext("/foo", new EchoHandler());
+        URI baseURI = new URI("https://127.0.0.1:" + port + "/foo/x");
+        server.createContext("/foo", new TestEchoHandler());
         server.start();
 
         RequestLimiter limiter = new RequestLimiter(40);
@@ -99,24 +134,32 @@
         HashMap<HttpRequest,byte[]> bodies = new HashMap<>();
 
         for (int i=0; i<REQUESTS; i++) {
-            byte[] buf = new byte[i+1];  // different size bodies
+            byte[] buf = new byte[(i+1)*CHUNK_SIZE+i+1];  // different size bodies
             rand.nextBytes(buf);
+            URI uri = new URI(baseURI.toString() + String.valueOf(i+1));
             HttpRequest r = HttpRequest.newBuilder(uri)
+                                       .header("XFixed", "true")
                                        .POST(fromByteArray(buf))
                                        .build();
             bodies.put(r, buf);
 
             results[i] =
                 limiter.whenOkToSend()
-                       .thenCompose((v) -> client.sendAsync(r, asByteArray()))
+                       .thenCompose((v) -> {
+                           System.out.println("Client: sendAsync: " + r.uri());
+                           return client.sendAsync(r, asByteArray());
+                       })
                        .thenCompose((resp) -> {
                            limiter.requestComplete();
                            if (resp.statusCode() != 200) {
                                String s = "Expected 200, got: " + resp.statusCode();
+                               System.out.println(s + " from "
+                                                  + resp.request().uri().getPath());
                                return completedWithIOException(s);
                            } else {
                                counter++;
-                               System.out.println("Result from " + counter);
+                               System.out.println("Result (" + counter + ") from "
+                                                   + resp.request().uri().getPath());
                            }
                            return CompletableFuture.completedStage(resp.body())
                                       .thenApply((b) -> new Pair<>(resp, b));
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/java/net/httpclient/ManyRequests2.java	Thu Jun 08 12:41:07 2017 +0100
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2017, 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.
+ */
+
+/*
+ * @test
+ * @bug 8087112 8180044
+ * @modules jdk.incubator.httpclient
+ *          java.logging
+ *          jdk.httpserver
+ * @library /lib/testlibrary/ /
+ * @build jdk.testlibrary.SimpleSSLContext EchoHandler
+ * @compile ../../../com/sun/net/httpserver/LogFilter.java
+ * @compile ../../../com/sun/net/httpserver/FileServerHandler.java
+ * @build ManyRequests ManyRequests2
+ * @run main/othervm/timeout=40 -Dtest.XFixed=true ManyRequests2
+ * @run main/othervm/timeout=40 -Dtest.XFixed=true -Dtest.insertDelay=true ManyRequests2
+ * @run main/othervm/timeout=40 -Dtest.XFixed=true -Dtest.chunkSize=64 ManyRequests2
+ * @run main/othervm/timeout=40 -Dtest.XFixed=true -Dtest.insertDelay=true -Dtest.chunkSize=64 ManyRequests2
+ * @summary Send a large number of requests asynchronously. The server echoes back using known content length.
+ */
+ // * @run main/othervm/timeout=40 -Djdk.httpclient.HttpClient.log=ssl ManyRequests
+
+public class ManyRequests2 {
+
+    public static void main(String[] args) throws Exception {
+        ManyRequests.main(args);
+    }
+}