Skip to content

Commit 72e1f7e

Browse files
committed
Avoid deadlock between SockJS heartbeat and XHR polling
Issue: SPR-14833
1 parent bf9083d commit 72e1f7e

File tree

2 files changed

+8
-23
lines changed

2 files changed

+8
-23
lines changed

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

+2-17
Original file line numberDiff line numberDiff line change
@@ -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,
@@ -209,14 +203,10 @@ public void handleInitialRequest(ServerHttpRequest request, ServerHttpResponse r
209203
this.frameFormat = frameFormat;
210204
this.asyncRequestControl = request.getAsyncRequestControl(response);
211205
this.asyncRequestControl.start(-1);
212-
213206
disableShallowEtagHeaderFilter(request);
214-
215207
// Let "our" handler know before sending the open frame to the remote handler
216208
delegateConnectionEstablished();
217-
218209
handleRequestInternal(request, response, true);
219-
220210
// Request might have been reset (e.g. polling sessions do after writing)
221211
this.readyToSend = isActive();
222212
}
@@ -252,9 +242,7 @@ public void handleSuccessiveRequest(ServerHttpRequest request, ServerHttpRespons
252242
this.frameFormat = frameFormat;
253243
this.asyncRequestControl = request.getAsyncRequestControl(response);
254244
this.asyncRequestControl.start(-1);
255-
256245
disableShallowEtagHeaderFilter(request);
257-
258246
handleRequestInternal(request, response, false);
259247
this.readyToSend = isActive();
260248
}
@@ -318,14 +306,11 @@ protected void disconnect(CloseStatus status) {
318306

319307
protected void resetRequest() {
320308
synchronized (this.responseLock) {
321-
322309
ServerHttpAsyncRequestControl control = this.asyncRequestControl;
323310
this.asyncRequestControl = null;
324311
this.readyToSend = false;
325312
this.response = null;
326-
327313
updateLastActiveTime();
328-
329314
if (control != null && !control.isCompleted()) {
330315
if (control.isStarted()) {
331316
try {

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ private enum State {NEW, OPEN, CLOSED}
9191

9292
protected final Log logger = LogFactory.getLog(getClass());
9393

94+
protected final Object responseLock = new Object();
95+
9496
private final String id;
9597

9698
private final SockJsServiceConfig config;
@@ -109,8 +111,6 @@ private enum State {NEW, OPEN, CLOSED}
109111

110112
private HeartbeatTask heartbeatTask;
111113

112-
private final Object heartbeatLock = new Object();
113-
114114
private volatile boolean heartbeatDisabled;
115115

116116

@@ -250,7 +250,7 @@ public void disableHeartbeat() {
250250
}
251251

252252
protected void sendHeartbeat() throws SockJsTransportFailureException {
253-
synchronized (this.heartbeatLock) {
253+
synchronized (this.responseLock) {
254254
if (isActive() && !this.heartbeatDisabled) {
255255
writeFrame(SockJsFrame.heartbeatFrame());
256256
scheduleHeartbeat();
@@ -262,7 +262,7 @@ protected void scheduleHeartbeat() {
262262
if (this.heartbeatDisabled) {
263263
return;
264264
}
265-
synchronized (this.heartbeatLock) {
265+
synchronized (this.responseLock) {
266266
cancelHeartbeat();
267267
if (!isActive()) {
268268
return;
@@ -277,7 +277,7 @@ protected void scheduleHeartbeat() {
277277
}
278278

279279
protected void cancelHeartbeat() {
280-
synchronized (this.heartbeatLock) {
280+
synchronized (this.responseLock) {
281281
if (this.heartbeatFuture != null) {
282282
if (logger.isTraceEnabled()) {
283283
logger.trace("Cancelling heartbeat in session " + getId());
@@ -445,7 +445,7 @@ private class HeartbeatTask implements Runnable {
445445

446446
@Override
447447
public void run() {
448-
synchronized (heartbeatLock) {
448+
synchronized (responseLock) {
449449
if (!this.expired) {
450450
try {
451451
sendHeartbeat();

0 commit comments

Comments
 (0)