Blob Blame History Raw
--- java/org/apache/tomcat/websocket/WsSession.java.org	2017-03-09 14:51:41.000000000 +0100
+++ java/org/apache/tomcat/websocket/WsSession.java	2018-07-18 17:37:46.853657200 +0200
@@ -595,8 +595,8 @@
             localEndpoint.onError(this, e);
         }
     }
-    
-    
+
+
     /**
      * Use protected so unit tests can access this method directly.
      */
@@ -635,29 +635,48 @@
      * {@link FutureToSendHandler} completes.
      */
     protected void registerFuture(FutureToSendHandler f2sh) {
-        boolean fail = false;
-        synchronized (stateLock) {
-            // If the session has already been closed the any registered futures
-            // will have been processed so the failure result for this future
-            // needs to be set here.
-            if (state == State.OPEN || f2sh.isCloseMessage()) {
-                // WebSocket session is open or this is the close message
-                futures.put(f2sh, f2sh);
-            } else if (f2sh.isDone()) {
-                // NO-OP. The future completed before the session closed so no
-                // need to register in case the session closes before it
-                // completes.
-            } else {
-                // Construct the exception outside of the sync block
-                fail = true;
-            }
+        // Ideally, this code should sync on stateLock so that the correct
+        // action is taken based on the current state of the connection.
+        // However, a sync on stateLock can't be used here as it will create the
+        // possibility of a dead-lock. See BZ 61183.
+        // Therefore, a slightly less efficient approach is used.
+
+        // Always register the future.
+        futures.put(f2sh, f2sh);
+
+        if (state == State.OPEN || f2sh.isCloseMessage()) {
+            // The session is open. The future has been registered with the open
+            // session. Normal processing continues.
+            return;
         }
 
-        if (fail) {
-            IOException ioe = new IOException(sm.getString("wsSession.messageFailed"));
-            SendResult sr = new SendResult(ioe);
-            f2sh.onResult(sr);
+        // The session is closed. The future may or may not have been registered
+        // in time for it to be processed during session closure.
+
+        if (f2sh.isDone()) {
+            // The future has completed. It is not known if the future was
+            // completed normally by the I/O layer or in error by doClose(). It
+            // doesn't matter which. There is nothing more to do here.
+            return;
         }
+
+        // The session is closed. The Future had not completed when last checked.
+        // There is a small timing window that means the Future may have been
+        // completed since the last check. There is also the possibility that
+        // the Future was not registered in time to be cleaned up during session
+        // close.
+        // Attempt to complete the Future with an error result as this ensures
+        // that the Future completes and any client code waiting on it does not
+        // hang. It is slightly inefficient since the Future may have been
+        // completed in another thread or another thread may be about to
+        // complete the Future but knowing if this is the case requires the sync
+        // on stateLock (see above).
+        // Note: If multiple attempts are made to complete the Future, the
+        //       second and subsequent attempts are ignored.
+
+        IOException ioe = new IOException(sm.getString("wsSession.messageFailed"));
+        SendResult sr = new SendResult(ioe);
+        f2sh.onResult(sr);
     }