diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java index 2321d8c9d76..8130921ea38 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java @@ -213,10 +213,10 @@ void deleteConnection(Http2Connection c) { private EOFException STOPPED; void stop() { - synchronized (this) {stopping = true;} if (debug.on()) debug.log("stopping"); STOPPED = new EOFException("HTTP/2 client stopped"); STOPPED.setStackTrace(new StackTraceElement[0]); + synchronized (this) {stopping = true;} do { connections.values().forEach(this::close); } while (!connections.isEmpty()); diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java index 02450c7dcf8..14fd7b6fc5b 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java @@ -28,6 +28,8 @@ import java.io.EOFException; import java.io.IOException; import java.io.UncheckedIOException; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; import java.net.InetSocketAddress; import java.net.ProtocolException; import java.net.URI; @@ -287,7 +289,10 @@ public void onMaxHeaderListSizeReached(long size, int maxHeaderListSize) throws } } - volatile boolean closed; + private static final int HALF_CLOSED_LOCAL = 1; + private static final int HALF_CLOSED_REMOTE = 2; + private static final int SHUTDOWN_REQUESTED = 4; + volatile int closedState; //------------------------------------- final HttpConnection connection; @@ -703,13 +708,15 @@ final int maxConcurrentServerInitiatedStreams() { } void close() { - Log.logTrace("Closing HTTP/2 connection: to {0}", connection.address()); - if (connection.channel().isOpen()) { - GoAwayFrame f = new GoAwayFrame(0, - ErrorFrame.NO_ERROR, - "Requested by user".getBytes(UTF_8)); - // TODO: set last stream. For now zero ok. - sendFrame(f); + if (markHalfClosedLocal()) { + if (connection.channel().isOpen()) { + Log.logTrace("Closing HTTP/2 connection: to {0}", connection.address()); + GoAwayFrame f = new GoAwayFrame(0, + ErrorFrame.NO_ERROR, + "Requested by user".getBytes(UTF_8)); + // TODO: set last stream. For now zero ok. + sendFrame(f); + } } } @@ -771,18 +778,17 @@ Throwable getRecordedCause() { } void shutdown(Throwable t) { - if (debug.on()) debug.log(() -> "Shutting down h2c (closed="+closed+"): " + t); - if (closed == true) return; - synchronized (this) { - if (closed == true) return; - closed = true; - } + int state = closedState; + if (debug.on()) debug.log(() -> "Shutting down h2c (state="+describeClosedState(state)+"): " + t); + if (!markShutdownRequested()) return; cause.compareAndSet(null, t); if (Log.errors()) { - if (!(t instanceof EOFException) || isActive()) { + if (t!= null && (!(t instanceof EOFException) || isActive())) { Log.logError(t); } else if (t != null) { Log.logError("Shutting down connection: {0}", t.getMessage()); + } else { + Log.logError("Shutting down connection"); } } client2.deleteConnection(this); @@ -966,7 +972,7 @@ private String checkMaxOrphanedHeadersExceeded(HeaderFrame hf) { } final void dropDataFrame(DataFrame df) { - if (closed) return; + if (isMarked(closedState, SHUTDOWN_REQUESTED)) return; if (debug.on()) { debug.log("Dropping data frame for stream %d (%d payload bytes)", df.streamid(), df.payloadLength()); @@ -976,7 +982,7 @@ final void dropDataFrame(DataFrame df) { final void ensureWindowUpdated(DataFrame df) { try { - if (closed) return; + if (isMarked(closedState, SHUTDOWN_REQUESTED)) return; int length = df.payloadLength(); if (length > 0) { windowUpdater.update(length); @@ -1076,7 +1082,8 @@ private void handleConnectionFrame(Http2Frame frame) } boolean isOpen() { - return !closed && connection.channel().isOpen(); + return !isMarked(closedState, SHUTDOWN_REQUESTED) + && connection.channel().isOpen(); } void resetStream(int streamid, int code) { @@ -1189,11 +1196,13 @@ private void protocolError(int errorCode, String msg) String protocolError = "protocol error" + (msg == null?"":(": " + msg)); ProtocolException protocolException = new ProtocolException(protocolError); - framesDecoder.close(protocolError); - subscriber.stop(protocolException); - if (debug.on()) debug.log("Sending GOAWAY due to " + protocolException); - GoAwayFrame frame = new GoAwayFrame(0, errorCode); - sendFrame(frame); + if (markHalfClosedLocal()) { + framesDecoder.close(protocolError); + subscriber.stop(protocolException); + if (debug.on()) debug.log("Sending GOAWAY due to " + protocolException); + GoAwayFrame frame = new GoAwayFrame(0, errorCode); + sendFrame(frame); + } shutdown(protocolException); } @@ -1226,9 +1235,11 @@ private void handlePing(PingFrame frame) private void handleGoAway(GoAwayFrame frame) throws IOException { - shutdown(new IOException( - connection.channel().getLocalAddress() - +": GOAWAY received")); + if (markHalfClosedLRemote()) { + shutdown(new IOException( + connection.channel().getLocalAddress() + + ": GOAWAY received")); + } } /** @@ -1342,7 +1353,7 @@ void putStream(Stream stream, int streamid) { // to prevent the SelectorManager thread from exiting until // the stream is closed. synchronized (this) { - if (!closed) { + if (!isMarked(closedState, SHUTDOWN_REQUESTED)) { if (debug.on()) { debug.log("Opened stream %d", streamid); } @@ -1353,7 +1364,6 @@ void putStream(Stream stream, int streamid) { } if (debug.on()) debug.log("connection closed: closing stream %d", stream); stream.cancel(); - } /** @@ -1491,7 +1501,7 @@ void sendFrame(Http2Frame frame) { } publisher.signalEnqueued(); } catch (IOException e) { - if (!closed) { + if (!isMarked(closedState, SHUTDOWN_REQUESTED)) { Log.logError(e); shutdown(e); } @@ -1509,7 +1519,7 @@ void sendDataFrame(DataFrame frame) { publisher.enqueue(encodeFrame(frame)); publisher.signalEnqueued(); } catch (IOException e) { - if (!closed) { + if (!isMarked(closedState, SHUTDOWN_REQUESTED)) { Log.logError(e); shutdown(e); } @@ -1527,7 +1537,7 @@ void sendUnorderedFrame(Http2Frame frame) { publisher.enqueueUnordered(encodeFrame(frame)); publisher.signalEnqueued(); } catch (IOException e) { - if (!closed) { + if (!isMarked(closedState, SHUTDOWN_REQUESTED)) { Log.logError(e); shutdown(e); } @@ -1693,4 +1703,60 @@ AbstractAsyncSSLConnection getConnection() { return connection; } } + + private boolean isMarked(int state, int mask) { + return (state & mask) == mask; + } + + private boolean markShutdownRequested() { + return markClosedState(SHUTDOWN_REQUESTED); + } + + private boolean markHalfClosedLocal() { + return markClosedState(HALF_CLOSED_LOCAL); + } + + private boolean markHalfClosedLRemote() { + return markClosedState(HALF_CLOSED_REMOTE); + } + + private boolean markClosedState(int flag) { + int state, desired; + do { + state = desired = closedState; + if ((state & flag) == flag) return false; + desired = state | flag; + } while (!CLOSED_STATE.compareAndSet(this, state, desired)); + return true; + } + + String describeClosedState(int state) { + if (state == 0) return "active"; + String desc = null; + if (isMarked(state, SHUTDOWN_REQUESTED)) { + desc = "shutdown"; + } + if (isMarked(state, HALF_CLOSED_LOCAL | HALF_CLOSED_REMOTE)) { + if (desc == null) return "closed"; + else return desc + "+closed"; + } + if (isMarked(state, HALF_CLOSED_LOCAL)) { + if (desc == null) return "half-closed-local"; + else return desc + "+half-closed-local"; + } + if (isMarked(state, HALF_CLOSED_REMOTE)) { + if (desc == null) return "half-closed-remote"; + else return desc + "+half-closed-remote"; + } + return "0x" + Integer.toString(state, 16); + } + + private static final VarHandle CLOSED_STATE; + static { + try { + CLOSED_STATE = MethodHandles.lookup().findVarHandle(Http2Connection.class, "closedState", int.class); + } catch (Exception x) { + throw new ExceptionInInitializerError(x); + } + } } diff --git a/test/jdk/java/net/httpclient/http2/NoBodyTest.java b/test/jdk/java/net/httpclient/http2/NoBodyTest.java index 2dc7508f5ee..4dcd99e2c87 100644 --- a/test/jdk/java/net/httpclient/http2/NoBodyTest.java +++ b/test/jdk/java/net/httpclient/http2/NoBodyTest.java @@ -26,7 +26,9 @@ * @bug 8087112 * @library /test/lib /test/jdk/java/net/httpclient/lib * @build jdk.test.lib.net.SimpleSSLContext jdk.httpclient.test.lib.http2.Http2TestServer - * @run testng/othervm -Djdk.httpclient.HttpClient.log=ssl,requests,responses,errors NoBodyTest + * @run testng/othervm -Djdk.httpclient.HttpClient.log=ssl,requests,responses,errors + * -Djdk.internal.httpclient.debug=true + * NoBodyTest */ import java.io.IOException;