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