Backport 041a12e65530b5832b4a500180c97a2a60e0dc51

Author: GoeLin

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());

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 @@ <T> void putStream(Stream<T> 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 @@ <T> void putStream(Stream<T> 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);
+        }
+    }
 }

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;