# HG changeset patch # User dfuchs # Date 1496922067 -3600 # Node ID fb3dbffad37b07d7a2ca09ad4cfc8dc8a535a7d1 # Parent 3c98842fddf75779b7abf3b7cd713b7888c72eb6 8180044: java/net/httpclient/ManyRequests.java failed due to timeout Summary: Fixes several race conditions observed while testing. Reviewed-by: michaelm, msheppar, prappo diff -r 3c98842fddf7 -r fb3dbffad37b jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java --- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java Thu Jun 08 12:24:13 2017 +0100 +++ b/jdk/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(); diff -r 3c98842fddf7 -r fb3dbffad37b jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java --- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java Thu Jun 08 12:24:13 2017 +0100 +++ b/jdk/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 asyncReceiver; private volatile Consumer errorReceiver; private volatile Supplier 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 whenReceivingResponse() { CompletableFuture cf = new MinimalFuture<>(); try { - client.registerEvent(new ReceiveResponseEvent(cf)); + ReceiveResponseEvent evt = new ReceiveResponseEvent(cf); + client.registerEvent(evt); } catch (IOException e) { cf.completeExceptionally(e); } diff -r 3c98842fddf7 -r fb3dbffad37b jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Stream.java --- a/jdk/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Stream.java Thu Jun 08 12:24:13 2017 +0100 +++ b/jdk/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); } diff -r 3c98842fddf7 -r fb3dbffad37b jdk/test/com/sun/net/httpserver/FileServerHandler.java --- a/jdk/test/com/sun/net/httpserver/FileServerHandler.java Thu Jun 08 12:24:13 2017 +0100 +++ b/jdk/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(); } } -} - diff -r 3c98842fddf7 -r fb3dbffad37b jdk/test/java/net/httpclient/ManyRequests.java --- a/jdk/test/java/net/httpclient/ManyRequests.java Thu Jun 08 12:24:13 2017 +0100 +++ b/jdk/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 bodies = new HashMap<>(); for (int i=0; i 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)); diff -r 3c98842fddf7 -r fb3dbffad37b jdk/test/java/net/httpclient/ManyRequests2.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/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); + } +}