Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition when inbound message handling fails and StompSubProtocolHandler sends ERROR frame [SPR-13326] #17911

Closed
spring-projects-issues opened this issue Aug 6, 2015 · 8 comments
Assignees
Labels
status: backported type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 6, 2015

Tong Chen opened SPR-13326 and commented

If for any reason such as some message parsing failed, StompSubProtocolHandler.sendErrorMessage is called and it ultimately calling StandardWebSocketSession.sendTextMessage to send the error msg back to client.

But from that method call I am getting exception:
"The remote endpoint was in state [TEXT_PARTIAL_WRITING] which is an invalid state for called method".

It looks like a race condition because when it is trying to use the native session to send msg back, the native session is also used by other normal messages.
Adding the following work around seems to fixed the problem.

@Override
protected void sendTextMessage(TextMessage message) throws IOException {
    Session nativeSession = getNativeSession();
    synchronized (nativeSession) {
        nativeSession.getBasicRemote().sendText(message.getPayload(), message.isLast());
    }
}

The message calling flow is quite complicated so I may have missed something obvious please let me know if my observation is flawed.


Issue Links:

  • #18029 Lock acquired on object responseLock in method sendMessageInternal of class AbstractHttpSockJsSession is not released in some error condition resulting in deadlock

Referenced from: commits 33f9ead, 7defbfc

Backported to: 4.1.8

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2015

Rossen Stoyanchev commented

What version of Spring Framework are you using and also what server/version?

The SubProtocolWebSocketHandler which delegates to StompSubProtocolHandler wraps every WebSocketSession with ConcurrentWebSocketSessionDecorator and that has been the case since 4.0.3. Can you provide a stack trace that will confirm if the session decoration is in place? Also are you using more or less standard @EnableWebSocketMessageBroker setup or something else?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2015

Tong Chen commented

I am using spring 4.1.5.RELEASE
And I am using WebSocketMessageBrokerConfigurationSupport (rather than @EnableWebSocketMessageBroker) and only configured
registry.addEndpoint("/dashboard").withSockJS()
and
registry.enableSimpleBroker("/topic/");
registry.setApplicationDestinationPrefixes("/");

Stacktrace
java.lang.IllegalStateException: The remote endpoint was in state [TEXT_PARTIAL_WRITING] which is an invalid state for called method
at org.apache.tomcat.websocket.WsRemoteEndpointImplBase$StateMachine.checkState(WsRemoteEndpointImplBase.java:1148)
at org.apache.tomcat.websocket.WsRemoteEndpointImplBase$StateMachine.textPartialStart(WsRemoteEndpointImplBase.java:1106)
at org.apache.tomcat.websocket.WsRemoteEndpointImplBase.sendPartialString(WsRemoteEndpointImplBase.java:224)
at org.apache.tomcat.websocket.WsRemoteEndpointBasic.sendText(WsRemoteEndpointBasic.java:49)
at org.springframework.web.socket.adapter.standard.StandardWebSocketSession.sendTextMessage(StandardWebSocketSession.java:199)
at org.springframework.web.socket.adapter.AbstractWebSocketSession.sendMessage(AbstractWebSocketSession.java:105)
at org.springframework.web.socket.sockjs.transport.session.WebSocketServerSockJsSession.writeFrameInternal(WebSocketServerSockJsSession.java:222)
at org.springframework.web.socket.sockjs.transport.session.AbstractSockJsSession.writeFrame(AbstractSockJsSession.java:325)
at org.springframework.web.socket.sockjs.transport.session.WebSocketServerSockJsSession.sendMessageInternal(WebSocketServerSockJsSession.java:212)
at org.springframework.web.socket.sockjs.transport.session.AbstractSockJsSession.sendMessage(AbstractSockJsSession.java:161)
at org.springframework.web.socket.messaging.StompSubProtocolHandler.sendErrorMessage(StompSubProtocolHandler.java:312)
at org.springframework.web.socket.messaging.StompSubProtocolHandler.handleMessageFromClient(StompSubProtocolHandler.java:276)
at org.springframework.web.socket.messaging.SubProtocolWebSocketHandler.handleMessage(SubProtocolWebSocketHandler.java:309)
at org.springframework.web.socket.handler.WebSocketHandlerDecorator.handleMessage(WebSocketHandlerDecorator.java:75)
at org.springframework.web.socket.handler.LoggingWebSocketHandlerDecorator.handleMessage(LoggingWebSocketHandlerDecorator.java:56)
at org.springframework.web.socket.handler.ExceptionWebSocketHandlerDecorator.handleMessage(ExceptionWebSocketHandlerDecorator.java:72)
at org.springframework.web.socket.sockjs.transport.session.AbstractSockJsSession.delegateMessages(AbstractSockJsSession.java:385)
at org.springframework.web.socket.sockjs.transport.session.WebSocketServerSockJsSession.handleMessage(WebSocketServerSockJsSession.java:194)
at org.springframework.web.socket.sockjs.transport.handler.SockJsWebSocketHandler.handleTextMessage(SockJsWebSocketHandler.java:92)
at org.springframework.web.socket.handler.AbstractWebSocketHandler.handleMessage(AbstractWebSocketHandler.java:43)
at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter.handleTextMessage(StandardWebSocketHandlerAdapter.java:112)
at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter.access$000(StandardWebSocketHandlerAdapter.java:42)
at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter$3.onMessage(StandardWebSocketHandlerAdapter.java:82)
at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter$3.onMessage(StandardWebSocketHandlerAdapter.java:79)
at org.apache.tomcat.websocket.WsFrameBase.sendMessageText(WsFrameBase.java:393)
at org.apache.tomcat.websocket.WsFrameBase.processDataText(WsFrameBase.java:494)
at org.apache.tomcat.websocket.WsFrameBase.processData(WsFrameBase.java:289)
at org.apache.tomcat.websocket.WsFrameBase.processInputBuffer(WsFrameBase.java:130)
at org.apache.tomcat.websocket.server.WsFrameServer.onDataAvailable(WsFrameServer.java:60)
at org.apache.tomcat.websocket.server.WsHttpUpgradeHandler$WsReadListener.onDataAvailable(WsHttpUpgradeHandler.java:203)
at org.apache.coyote.http11.upgrade.AbstractServletInputStream.onDataAvailable(AbstractServletInputStream.java:198)
at org.apache.coyote.http11.upgrade.AbstractProcessor.upgradeDispatch(AbstractProcessor.java:96)
at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:654)
at org.apache.coyote.http11.Http11NioProtocol$Http11ConnectionHandler.process(Http11NioProtocol.java:223)
at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1558)
at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1515)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
at java.lang.Thread.run(Thread.java:745)

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2015

Rossen Stoyanchev commented

Oops, looks like we are only passing the concurrent session decorator to SubProtocolHandler for outbound messages and never did so for inbound messages for which we don't typically use the session unless inbound message handling fails and we try to send an ERROR frame to the client. Thanks for reporting!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2015

Tong Chen commented

Thanks Rossen, in the mean time, can you suggest a nicer workaround (rather than the synchronized block I put in)?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 7, 2015

Rossen Stoyanchev commented

Since you're already using WebSocketMessageBrokerConfigurationSupport, you could override subProtocolWebSocketHandler and return a SubProtocolWebSocketHandler sub-class overriding handleMessage(WebSocketSession session, WebSocketMessage<?> message) and delegating to super with a decorated session. The only catch is that the map with decorated sessions is a private field in SubProtocolWebSocketHandler so you'll have to get it with reflection I'm afraid (e.g. using ReflectionUtils#makeAccessible).

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 9, 2015

Chandan commented

Is this issue Only with Stomp? I am observing similar issue while using SockJs support -client is sockjs which uses JSON RPC protocol. For some conditions it is resulting in Race Conditions:
Waiting for Monitor Lock on java/lang/Object@0x000000000C9465E0
at org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.sendMessageInternal(AbstractHttpSockJsSession.java:279(Compiled Code))
at org/springframework/web/socket/sockjs/transport/session/AbstractSockJsSession.sendMessage(AbstractSockJsSession.java:161(Compiled Code))

AbstractHttpSockJsSession acquires a lock while sending message :
@Override
protected final void sendMessageInternal(String message) throws SockJsTransportFailureException {
synchronized (this.responseLock) {

For this reason, I was not using lock in my handlers. When a synchronised block, it alleviates the issue

@Override
public void handleTextMessage(WebSocketSession session,
TextMessage message) {
synchronized (engine) {
engine.handle(message.getPayload());
}
}

I am using 4.1.5 but I am unable to assess the exact cause of race condition. If possible, please confirm if this issue is for HTTP handlers as well -then I can propose a lib upgrade (We use platform so it might be a tedious process and needs concrete validations)

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 21, 2015

Andre Gonçalves commented

@Chandan I'm having the same issue with SockJS. I'm on 4.2.7

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 21, 2015

Rossen Stoyanchev commented

Are you at the WebSocket/SockJS level, i.e. not using the sub-protocol support? The STOMP support also takes care of sending messages concurrently. If using a WebSocketSession directly you need take care of synchronizing the sends. This shouldn't be too hard however if you wrap the session. See my response under #18029.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: backported type: bug
Projects
None yet
Development

No branches or pull requests

2 participants