# HG changeset patch # User michaelm # Date 1512156334 0 # Node ID 2d423c9b73bbab29946aeeb28c1f0ae388ae1440 # Parent 8e09e6396acde9aaf52dbe9e1f5da5df157c6c69 http-client-branch: Fixed some Http2TestServer issues diff -r 8e09e6396acd -r 2d423c9b73bb test/jdk/java/net/httpclient/http2/BasicTest.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 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 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 diff -r 8e09e6396acd -r 2d423c9b73bb test/jdk/java/net/httpclient/http2/server/Http2EchoHandler.java --- 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(); diff -r 8e09e6396acd -r 2d423c9b73bb test/jdk/java/net/httpclient/http2/server/Http2TestServer.java --- 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; } } diff -r 8e09e6396acd -r 2d423c9b73bb test/jdk/java/net/httpclient/http2/server/Http2TestServerConnection.java --- 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 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 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 { diff -r 8e09e6396acd -r 2d423c9b73bb test/jdk/java/net/httpclient/http2/server/Queue.java --- 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) {