Avoid locking in WebSocket session "close" callback
When processing a "close" notification from the server make an effort to cancel any outstanding heartbeat but avoid going as far as acquiring the responseLock since the server itself may already hold a lock of its own leading to a potential deadlock. The heartbeat task is now also further protected with an isClosed() check in case the heartbeat does not get cancelled in a concurrent scenario. Issue: SPR-14917
This commit is contained in:
@@ -396,7 +396,12 @@ public abstract class AbstractSockJsSession implements SockJsSession {
|
||||
if (!isClosed()) {
|
||||
try {
|
||||
updateLastActiveTime();
|
||||
cancelHeartbeat();
|
||||
// Avoid cancelHeartbeat() and responseLock within server "close" callback
|
||||
ScheduledFuture<?> future = this.heartbeatFuture;
|
||||
if (future != null) {
|
||||
this.heartbeatFuture = null;
|
||||
future.cancel(false);
|
||||
}
|
||||
}
|
||||
finally {
|
||||
this.state = State.CLOSED;
|
||||
@@ -446,7 +451,7 @@ public abstract class AbstractSockJsSession implements SockJsSession {
|
||||
@Override
|
||||
public void run() {
|
||||
synchronized (responseLock) {
|
||||
if (!this.expired) {
|
||||
if (!this.expired && !isClosed()) {
|
||||
try {
|
||||
sendHeartbeat();
|
||||
}
|
||||
|
||||
@@ -144,7 +144,6 @@ public class SockJsSessionTests extends AbstractSockJsSessionTests<TestSockJsSes
|
||||
|
||||
assertClosed();
|
||||
assertEquals(1, this.session.getNumberOfLastActiveTimeUpdates());
|
||||
assertTrue(this.session.didCancelHeartbeat());
|
||||
verify(this.webSocketHandler).afterConnectionClosed(this.session, CloseStatus.GOING_AWAY);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user