Skip to content

Commit 4ce0e6b

Browse files
committed
Avoid deadlock between SockJS heartbeat and XHR polling
Issue: SPR-14833 (cherry picked from commit 1c80d2a)
1 parent 0d6dc57 commit 4ce0e6b

File tree

2 files changed

+10
-24
lines changed

2 files changed

+10
-24
lines changed

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java

+3-18
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -51,6 +51,7 @@
5151
*/
5252
public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession {
5353

54+
private final Queue<String> messageCache;
5455

5556
private volatile URI uri;
5657

@@ -64,20 +65,13 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession {
6465

6566
private volatile String acceptedProtocol;
6667

67-
6868
private volatile ServerHttpResponse response;
6969

7070
private volatile SockJsFrameFormat frameFormat;
7171

72-
7372
private volatile ServerHttpAsyncRequestControl asyncRequestControl;
7473

75-
private final Object responseLock = new Object();
76-
77-
private volatile boolean readyToSend;
78-
79-
80-
private final Queue<String> messageCache;
74+
private boolean readyToSend;
8175

8276

8377
public AbstractHttpSockJsSession(String id, SockJsServiceConfig config,
@@ -205,14 +199,10 @@ public void handleInitialRequest(ServerHttpRequest request, ServerHttpResponse r
205199
this.frameFormat = frameFormat;
206200
this.asyncRequestControl = request.getAsyncRequestControl(response);
207201
this.asyncRequestControl.start(-1);
208-
209202
disableShallowEtagHeaderFilter(request);
210-
211203
// Let "our" handler know before sending the open frame to the remote handler
212204
delegateConnectionEstablished();
213-
214205
handleRequestInternal(request, response, true);
215-
216206
// Request might have been reset (e.g. polling sessions do after writing)
217207
this.readyToSend = isActive();
218208
}
@@ -248,9 +238,7 @@ public void handleSuccessiveRequest(ServerHttpRequest request, ServerHttpRespons
248238
this.frameFormat = frameFormat;
249239
this.asyncRequestControl = request.getAsyncRequestControl(response);
250240
this.asyncRequestControl.start(-1);
251-
252241
disableShallowEtagHeaderFilter(request);
253-
254242
handleRequestInternal(request, response, false);
255243
this.readyToSend = isActive();
256244
}
@@ -322,14 +310,11 @@ protected void disconnect(CloseStatus status) {
322310

323311
protected void resetRequest() {
324312
synchronized (this.responseLock) {
325-
326313
ServerHttpAsyncRequestControl control = this.asyncRequestControl;
327314
this.asyncRequestControl = null;
328315
this.readyToSend = false;
329316
this.response = null;
330-
331317
updateLastActiveTime();
332-
333318
if (control != null && !control.isCompleted()) {
334319
if (control.isStarted()) {
335320
try {

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import org.apache.commons.logging.Log;
3232
import org.apache.commons.logging.LogFactory;
33+
3334
import org.springframework.core.NestedCheckedException;
3435
import org.springframework.util.Assert;
3536
import org.springframework.web.socket.CloseStatus;
@@ -90,6 +91,8 @@ private enum State {NEW, OPEN, CLOSED}
9091

9192
protected final Log logger = LogFactory.getLog(getClass());
9293

94+
protected final Object responseLock = new Object();
95+
9396
private final String id;
9497

9598
private final SockJsServiceConfig config;
@@ -108,8 +111,6 @@ private enum State {NEW, OPEN, CLOSED}
108111

109112
private HeartbeatTask heartbeatTask;
110113

111-
private final Object heartbeatLock = new Object();
112-
113114
private volatile boolean heartbeatDisabled;
114115

115116

@@ -249,7 +250,7 @@ public void disableHeartbeat() {
249250
}
250251

251252
public void sendHeartbeat() throws SockJsTransportFailureException {
252-
synchronized (this.heartbeatLock) {
253+
synchronized (this.responseLock) {
253254
if (isActive() && !this.heartbeatDisabled) {
254255
writeFrame(SockJsFrame.heartbeatFrame());
255256
scheduleHeartbeat();
@@ -261,7 +262,7 @@ protected void scheduleHeartbeat() {
261262
if (this.heartbeatDisabled) {
262263
return;
263264
}
264-
synchronized (this.heartbeatLock) {
265+
synchronized (this.responseLock) {
265266
cancelHeartbeat();
266267
if (!isActive()) {
267268
return;
@@ -276,7 +277,7 @@ protected void scheduleHeartbeat() {
276277
}
277278

278279
protected void cancelHeartbeat() {
279-
synchronized (this.heartbeatLock) {
280+
synchronized (this.responseLock) {
280281
if (this.heartbeatFuture != null) {
281282
if (logger.isTraceEnabled()) {
282283
logger.trace("Cancelling heartbeat in session " + getId());
@@ -444,7 +445,7 @@ private class HeartbeatTask implements Runnable {
444445

445446
@Override
446447
public void run() {
447-
synchronized (heartbeatLock) {
448+
synchronized (responseLock) {
448449
if (!this.expired) {
449450
try {
450451
sendHeartbeat();

0 commit comments

Comments
 (0)