Issue #3734 - throw ISE for WebSocket suspend after close (#4095)

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-websocket/jetty-websocket-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java

@@ -48,7 +48,8 @@ public class JettyWebSocketFrameHandler implements FrameHandler
     {
         DEMANDING,
         SUSPENDING,
-        SUSPENDED
+        SUSPENDED,
+        CLOSED
     }
 
     private final Logger log;
@@ -191,7 +192,6 @@ public class JettyWebSocketFrameHandler implements FrameHandler
                     state = SuspendState.SUSPENDED;
                     return;
 
-                case SUSPENDED:
                 default:
                     throw new IllegalStateException();
             }
@@ -280,6 +280,12 @@ public class JettyWebSocketFrameHandler implements FrameHandler
     @Override
     public void onClosed(CloseStatus closeStatus, Callback callback)
     {
+        synchronized (this)
+        {
+            // We are now closed and cannot suspend or resume
+            state = SuspendState.CLOSED;
+        }
+
         try
         {
             if (closeHandle != null)
@@ -405,18 +411,15 @@ public class JettyWebSocketFrameHandler implements FrameHandler
                     state = SuspendState.SUSPENDING;
                     break;
 
-                case SUSPENDED:
-                case SUSPENDING:
-                    throw new IllegalStateException("Already Suspended");
-
                 default:
-                    throw new IllegalStateException();
+                    throw new IllegalStateException(state.name());
             }
         }
     }
 
     public void resume()
     {
+        boolean needDemand = false;
         Runnable delayedFrame = null;
         synchronized (this)
         {
@@ -426,6 +429,7 @@ public class JettyWebSocketFrameHandler implements FrameHandler
                     throw new IllegalStateException("Already Resumed");
 
                 case SUSPENDED:
+                    needDemand = true;
                     delayedFrame = delayedOnFrame;
                     delayedOnFrame = null;
                     state = SuspendState.DEMANDING;
@@ -438,15 +442,18 @@ public class JettyWebSocketFrameHandler implements FrameHandler
                     break;
 
                 default:
-                    throw new IllegalStateException();
+                    throw new IllegalStateException(state.name());
             }
         }
 
+        if (needDemand)
+        {
             if (delayedFrame != null)
                 delayedFrame.run();
             else
                 session.getCoreSession().demand(1);
         }
+    }
 
     private void demand()
     {
@@ -459,15 +466,12 @@ public class JettyWebSocketFrameHandler implements FrameHandler
                     demand = true;
                     break;
 
-                case SUSPENDED:
-                    throw new IllegalStateException("Suspended");
-
                 case SUSPENDING:
                     state = SuspendState.SUSPENDED;
                     break;
 
                 default:
-                    throw new IllegalStateException();
+                    throw new IllegalStateException(state.name());
             }
         }
 

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/SuspendResumeTest.java

@@ -36,7 +36,6 @@ import org.eclipse.jetty.websocket.server.JettyWebSocketServletFactory;
 import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -170,7 +169,6 @@ public class SuspendResumeTest
         assertNull(serverSocket.error);
     }
 
-    @Disabled
     @Test
     public void testSuspendAfterClose() throws Exception
     {

jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java

@@ -470,7 +470,7 @@ public class WebSocketCoreSession implements IncomingFrames, FrameHandler.CoreSe
         if (!demanding)
             throw new IllegalStateException("FrameHandler is not demanding: " + this);
         if (!sessionState.isInputOpen())
-            throw new IllegalStateException("FrameHandler input not open: " + this); // TODO Perhaps this is a NOOP?
+            throw new IllegalStateException("FrameHandler input not open: " + this);
         connection.demand(n);
     }