http-client-branch: Fixed some Http2TestServer issues http-client-branch
authormichaelm
Fri, 01 Dec 2017 19:25:34 +0000 (2017-12-01)
branchhttp-client-branch
changeset 55941 2d423c9b73bb
parent 55940 8e09e6396acd
child 55942 8d4770c22b63
http-client-branch: Fixed some Http2TestServer issues
test/jdk/java/net/httpclient/http2/BasicTest.java
test/jdk/java/net/httpclient/http2/server/Http2EchoHandler.java
test/jdk/java/net/httpclient/http2/server/Http2TestServer.java
test/jdk/java/net/httpclient/http2/server/Http2TestServerConnection.java
test/jdk/java/net/httpclient/http2/server/Queue.java
--- a/test/jdk/java/net/httpclient/http2/BasicTest.java	Fri Dec 01 09:40:02 2017 +0000
+++ b/test/jdk/java/net/httpclient/http2/BasicTest.java	Fri Dec 01 19:25:34 2017 +0000
@@ -118,6 +118,8 @@
     public static void test() throws Exception {
         try {
             initialize();
+            warmup(false);
+            warmup(true);
             simpleTest(false, false);
             simpleTest(false, true);
             simpleTest(true, false);
@@ -218,8 +220,7 @@
     }
 
     static void paramsTest() throws Exception {
-        Http2TestServer server = new Http2TestServer(true, 0, serverExec, sslContext);
-        server.addHandler((t -> {
+        httpsServer.addHandler((t -> {
             SSLSession s = t.getSSLSession();
             String prot = s.getProtocol();
             if (prot.equals("TLSv1.2")) {
@@ -229,9 +230,7 @@
                 t.sendResponseHeaders(500, -1);
             }
         }), "/");
-        server.start();
-        int port = server.getAddress().getPort();
-        URI u = new URI("https://127.0.0.1:"+port+"/foo");
+        URI u = new URI("https://127.0.0.1:"+httpsPort+"/foo");
         HttpClient client = getClient();
         HttpRequest req = HttpRequest.newBuilder(u).build();
         HttpResponse<String> resp = client.send(req, asString());
@@ -243,8 +242,8 @@
         System.err.println("paramsTest: DONE");
     }
 
-    static void simpleTest(boolean secure, boolean ping) throws Exception {
-        URI uri = getURI(secure, ping);
+    static void warmup(boolean secure) throws Exception {
+        URI uri = getURI(secure);
         System.err.println("Request to " + uri);
 
         // Do a simple warmup request
@@ -254,15 +253,17 @@
                                      .POST(fromString(SIMPLE_STRING))
                                      .build();
         HttpResponse<String> response = client.send(req, asString());
+        checkStatus(200, response.statusCode());
+        String responseBody = response.body();
         HttpHeaders h = response.headers();
-
-        checkStatus(200, response.statusCode());
-
-        String responseBody = response.body();
         checkStrings(SIMPLE_STRING, responseBody);
-
         checkStrings(h.firstValue("x-hello").get(), "world");
         checkStrings(h.firstValue("x-bye").get(), "universe");
+    }
+
+    static void simpleTest(boolean secure, boolean ping) throws Exception {
+        URI uri = getURI(secure, ping);
+        System.err.println("Request to " + uri);
 
         // Do loops asynchronously
 
--- a/test/jdk/java/net/httpclient/http2/server/Http2EchoHandler.java	Fri Dec 01 09:40:02 2017 +0000
+++ b/test/jdk/java/net/httpclient/http2/server/Http2EchoHandler.java	Fri Dec 01 19:25:34 2017 +0000
@@ -31,7 +31,7 @@
     public void handle(Http2TestExchange t)
             throws IOException {
         try {
-            System.err.println("EchoHandler received request to " + t.getRequestURI());
+            System.err.printf("EchoHandler received request to %s from %s\n", t.getRequestURI(), t.getRemoteAddress());
             InputStream is = t.getRequestBody();
             HttpHeadersImpl map = t.getRequestHeaders();
             HttpHeadersImpl map1 = t.getResponseHeaders();
--- a/test/jdk/java/net/httpclient/http2/server/Http2TestServer.java	Fri Dec 01 09:40:02 2017 +0000
+++ b/test/jdk/java/net/httpclient/http2/server/Http2TestServer.java	Fri Dec 01 19:25:34 2017 +0000
@@ -35,6 +35,7 @@
 import javax.net.ssl.SSLServerSocket;
 import javax.net.ssl.SSLServerSocketFactory;
 import javax.net.ssl.SNIServerName;
+import jdk.incubator.http.internal.frame.ErrorFrame;
 
 /**
  * Waits for incoming TCP connections from a client and establishes
@@ -172,8 +173,9 @@
     public void stop() {
         // TODO: clean shutdown GoAway
         stopping = true;
+        System.err.printf("Server stopping %d connections\n", connections.size());
         for (Http2TestServerConnection connection : connections.values()) {
-            connection.close();
+            connection.close(ErrorFrame.NO_ERROR);
         }
         try {
             server.close();
@@ -223,7 +225,7 @@
                         // and if so then the client might wait
                         // forever.
                         connections.remove(addr, c);
-                        c.close();
+                        c.close(ErrorFrame.PROTOCOL_ERROR);
                         throw e;
                     }
                 }
--- a/test/jdk/java/net/httpclient/http2/server/Http2TestServerConnection.java	Fri Dec 01 09:40:02 2017 +0000
+++ b/test/jdk/java/net/httpclient/http2/server/Http2TestServerConnection.java	Fri Dec 01 19:25:34 2017 +0000
@@ -85,6 +85,8 @@
         Sentinel() { super(-1,-1);}
     }
 
+    static final Sentinel sentinel = new Sentinel();
+
     class PingRequest {
         final byte[] pingData;
         final long pingStamp;
@@ -114,8 +116,6 @@
         }
     }
 
-    static Sentinel sentinel;
-
     Http2TestServerConnection(Http2TestServer server,
                               Socket socket,
                               Http2TestExchangeSupplier exchangeSupplier)
@@ -159,6 +159,13 @@
         return ping.response();
     }
 
+    void goAway(int error) throws IOException {
+        int laststream = nextstream >= 3 ? nextstream - 2 : 1;
+
+        GoAwayFrame go = new GoAwayFrame(laststream, error);
+        outputQ.put(go);
+    }
+
     /**
      * Returns the first PingRequest from Queue
      */
@@ -173,7 +180,7 @@
     void handlePing(PingFrame ping) throws IOException {
         if (ping.streamid() != 0) {
             System.err.println("Invalid ping received");
-            close();
+            close(ErrorFrame.PROTOCOL_ERROR);
             return;
         }
         if (ping.getFlag(PingFrame.ACK)) {
@@ -181,7 +188,7 @@
             PingRequest request = getNextRequest();
             if (request == null) {
                 System.err.println("Invalid ping ACK received");
-                close();
+                close(ErrorFrame.PROTOCOL_ERROR);
                 return;
             } else if (!Arrays.equals(request.pingData, ping.getData())) {
                 request.fail(new RuntimeException("Wrong ping data in ACK"));
@@ -231,15 +238,25 @@
         sock.getSession(); // blocks until handshake done
     }
 
-    void close() {
+    void closeIncoming() {
+        close(-1);
+    }
+
+    void close(int error) {
+        if (stopping)
+            return;
         stopping = true;
+        System.err.printf("Server connection to %s stopping. %d streams\n",
+            socket.getRemoteSocketAddress().toString(), streams.size());
         streams.forEach((i, q) -> {
-            q.close();
+            q.orderlyClose();
         });
         try {
+            if (error != -1)
+                goAway(error);
+            outputQ.orderlyClose();
             socket.close();
-            // TODO: put a reset on each stream
-        } catch (IOException e) {
+        } catch (Exception e) {
         }
     }
 
@@ -321,8 +338,21 @@
             nextstream = 3;
         }
 
-        exec.submit(this::readLoop);
-        exec.submit(this::writeLoop);
+        (new ConnectionThread("readLoop", this::readLoop)).start();
+        (new ConnectionThread("writeLoop", this::writeLoop)).start();
+    }
+
+    class ConnectionThread extends Thread {
+        final Runnable r;
+        ConnectionThread(String name, Runnable r) {
+            setName(name);
+            setDaemon(true);
+            this.r = r;
+        }
+
+        public void run() {
+            r.run();
+        }
     }
 
     private void writeFrame(Http2Frame frame) throws IOException {
@@ -369,7 +399,7 @@
             return;
         } else if (f instanceof GoAwayFrame) {
             System.err.println("Closing: "+ f.toString());
-            close();
+            close(ErrorFrame.NO_ERROR);
         } else if (f instanceof PingFrame) {
             handlePing((PingFrame)f);
         } else
@@ -569,7 +599,11 @@
     void readLoop() {
         try {
             while (!stopping) {
-                Http2Frame frame = readFrame();
+                Http2Frame frame = readFrameImpl();
+                if (frame == null) {
+                    closeIncoming();
+                    return;
+                }
                 //System.err.printf("TestServer: received frame %s\n", frame);
                 int stream = frame.streamid();
                 if (stream == 0) {
@@ -625,7 +659,7 @@
                 System.err.println("Http server reader thread shutdown");
                 e.printStackTrace();
             }
-            close();
+            close(ErrorFrame.PROTOCOL_ERROR);
         }
     }
 
@@ -667,6 +701,8 @@
                 Http2Frame frame;
                 try {
                     frame = outputQ.take();
+                    if (stopping)
+                        break;
                 } catch(IOException x) {
                     if (stopping && x.getCause() instanceof InterruptedException) {
                         break;
@@ -742,27 +778,46 @@
     }
 
     private Http2Frame readFrame() throws IOException {
-        byte[] buf = new byte[9];
-        if (is.readNBytes(buf, 0, 9) != 9)
-            throw new IOException("readFrame: connection closed");
-        int len = 0;
-        for (int i = 0; i < 3; i++) {
-            int n = buf[i] & 0xff;
-            //System.err.println("n = " + n);
-            len = (len << 8) + n;
+        Http2Frame f = readFrameImpl();
+        if (f == null)
+            throw new IOException("connection closed");
+        return f;
+    }
+
+    // does not throw an exception for EOF
+    private Http2Frame readFrameImpl() throws IOException {
+        try {
+            byte[] buf = new byte[9];
+            int ret;
+            ret=is.readNBytes(buf, 0, 9);
+            if (ret == 0) {
+                return null;
+            } else if (ret != 9) {
+                throw new IOException("readFrame: connection closed");
+            }
+            int len = 0;
+            for (int i = 0; i < 3; i++) {
+                int n = buf[i] & 0xff;
+                //System.err.println("n = " + n);
+                len = (len << 8) + n;
+            }
+            byte[] rest = new byte[len];
+            int n = is.readNBytes(rest, 0, len);
+            if (n != len)
+                throw new IOException("Error reading frame");
+            List<Http2Frame> frames = new ArrayList<>();
+            FramesDecoder reader = new FramesDecoder(frames::add);
+            reader.decode(ByteBuffer.wrap(buf));
+            reader.decode(ByteBuffer.wrap(rest));
+            if (frames.size()!=1)
+                throw new IOException("Expected 1 frame got "+frames.size()) ;
+
+            return frames.get(0);
+        } catch (IOException ee) {
+            if (stopping)
+                return null;
+            throw ee;
         }
-        byte[] rest = new byte[len];
-        int n = is.readNBytes(rest, 0, len);
-        if (n != len)
-            throw new IOException("Error reading frame");
-        List<Http2Frame> frames = new ArrayList<>();
-        FramesDecoder reader = new FramesDecoder(frames::add);
-        reader.decode(ByteBuffer.wrap(buf));
-        reader.decode(ByteBuffer.wrap(rest));
-        if (frames.size()!=1)
-            throw new IOException("Expected 1 frame got "+frames.size()) ;
-
-        return frames.get(0);
     }
 
     void sendSettingsFrame() throws IOException {
--- a/test/jdk/java/net/httpclient/http2/server/Queue.java	Fri Dec 01 09:40:02 2017 +0000
+++ b/test/jdk/java/net/httpclient/http2/server/Queue.java	Fri Dec 01 19:25:34 2017 +0000
@@ -36,8 +36,6 @@
     private boolean closed = false;
     private boolean closing = false;
     private Throwable exception = null;
-    private Runnable callback;
-    private boolean callbackDisabled = false;
     private int waiters; // true if someone waiting
     private final T closeSentinel;
 
@@ -59,16 +57,6 @@
         if (waiters > 0) {
             notifyAll();
         }
-
-        if (callbackDisabled) {
-            return;
-        }
-
-        if (q.size() > 0 && callback != null) {
-            // Note: calling callback while holding the lock is
-            // dangerous and may lead to deadlocks.
-            callback.run();
-        }
     }
 
     // Other close() variants are immediate and abortive
@@ -77,6 +65,7 @@
     public synchronized void orderlyClose() {
         if (closing || closed)
             return;
+
         try {
             put(closeSentinel);
         } catch (IOException e) {
@@ -87,6 +76,8 @@
 
     @Override
     public synchronized void close() {
+        if (closed)
+            return;
         closed = true;
         notifyAll();
     }
@@ -123,6 +114,7 @@
             if (item.equals(closeSentinel)) {
                 closed = true;
                 assert q.isEmpty();
+                return null;
             }
             return item;
         } catch (InterruptedException ex) {