From 4ce0e6b3c3311c0f2d2ff1a67401a696ba4f8da5 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 28 Oct 2016 23:34:19 +0200 Subject: [PATCH] Avoid deadlock between SockJS heartbeat and XHR polling Issue: SPR-14833 (cherry picked from commit 1c80d2a) --- .../session/AbstractHttpSockJsSession.java | 21 +++---------------- .../session/AbstractSockJsSession.java | 13 ++++++------ 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java index df0e2771c4..92da22d701 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -51,6 +51,7 @@ import org.springframework.web.socket.sockjs.transport.SockJsServiceConfig; */ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { + private final Queue messageCache; private volatile URI uri; @@ -64,20 +65,13 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { private volatile String acceptedProtocol; - private volatile ServerHttpResponse response; private volatile SockJsFrameFormat frameFormat; - private volatile ServerHttpAsyncRequestControl asyncRequestControl; - private final Object responseLock = new Object(); - - private volatile boolean readyToSend; - - - private final Queue messageCache; + private boolean readyToSend; public AbstractHttpSockJsSession(String id, SockJsServiceConfig config, @@ -205,14 +199,10 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { this.frameFormat = frameFormat; this.asyncRequestControl = request.getAsyncRequestControl(response); this.asyncRequestControl.start(-1); - disableShallowEtagHeaderFilter(request); - // Let "our" handler know before sending the open frame to the remote handler delegateConnectionEstablished(); - handleRequestInternal(request, response, true); - // Request might have been reset (e.g. polling sessions do after writing) this.readyToSend = isActive(); } @@ -248,9 +238,7 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { this.frameFormat = frameFormat; this.asyncRequestControl = request.getAsyncRequestControl(response); this.asyncRequestControl.start(-1); - disableShallowEtagHeaderFilter(request); - handleRequestInternal(request, response, false); this.readyToSend = isActive(); } @@ -322,14 +310,11 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { protected void resetRequest() { synchronized (this.responseLock) { - ServerHttpAsyncRequestControl control = this.asyncRequestControl; this.asyncRequestControl = null; this.readyToSend = false; this.response = null; - updateLastActiveTime(); - if (control != null && !control.isCompleted()) { if (control.isStarted()) { try { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java index d9ca92960e..7211c97049 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.java @@ -30,6 +30,7 @@ import java.util.concurrent.ScheduledFuture; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.core.NestedCheckedException; import org.springframework.util.Assert; import org.springframework.web.socket.CloseStatus; @@ -90,6 +91,8 @@ public abstract class AbstractSockJsSession implements SockJsSession { protected final Log logger = LogFactory.getLog(getClass()); + protected final Object responseLock = new Object(); + private final String id; private final SockJsServiceConfig config; @@ -108,8 +111,6 @@ public abstract class AbstractSockJsSession implements SockJsSession { private HeartbeatTask heartbeatTask; - private final Object heartbeatLock = new Object(); - private volatile boolean heartbeatDisabled; @@ -249,7 +250,7 @@ public abstract class AbstractSockJsSession implements SockJsSession { } public void sendHeartbeat() throws SockJsTransportFailureException { - synchronized (this.heartbeatLock) { + synchronized (this.responseLock) { if (isActive() && !this.heartbeatDisabled) { writeFrame(SockJsFrame.heartbeatFrame()); scheduleHeartbeat(); @@ -261,7 +262,7 @@ public abstract class AbstractSockJsSession implements SockJsSession { if (this.heartbeatDisabled) { return; } - synchronized (this.heartbeatLock) { + synchronized (this.responseLock) { cancelHeartbeat(); if (!isActive()) { return; @@ -276,7 +277,7 @@ public abstract class AbstractSockJsSession implements SockJsSession { } protected void cancelHeartbeat() { - synchronized (this.heartbeatLock) { + synchronized (this.responseLock) { if (this.heartbeatFuture != null) { if (logger.isTraceEnabled()) { logger.trace("Cancelling heartbeat in session " + getId()); @@ -444,7 +445,7 @@ public abstract class AbstractSockJsSession implements SockJsSession { @Override public void run() { - synchronized (heartbeatLock) { + synchronized (responseLock) { if (!this.expired) { try { sendHeartbeat();