Lock acquired on object responseLock in method sendMessageInternal of class AbstractHttpSockJsSession is not released in some error condition resulting in deadlock [SPR-13449]

[spring-projects-issues]

Chandan opened SPR-13449 and commented

We are using spring version 4.1.5 and container is websphere(WAS). On server side we are using webscoket with SockJs -need fallback mechanism as WAS do not support websockets. Client is sockjs client -but instead of using messaging protocol (STOMP) over socket, we use JSON RPC. there could be more than 10 XHR requests per second. While doing some more load testing and negative scenarios -like abruptly closing browsers, idle period in browsers etc. we run in scenario where one of thread to send message acquire lock on object responseLock in method sendMessageInternal of class AbstractHttpSockJsSession -resulting in blocked state.
As perconnection handler is being used, subsequent requests are served but over period cpu usage increases as number of blocked threads increase.

Waiting for Monitor Lock on java/lang/Object@0x0000000022416F08 (This is object lock while sending message)
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)) 
at com/avaya/cecobrowsega/cobrowse/service/handler/AgentConnectionHandlerImpl.send(AgentConnectionHandlerImpl.java:579(Compiled Code)) 

The send message is like this

@Override
public void send(String data) throws IOException {
    webSocketSession.sendMessage(new TextMessage(data));
}

synchronzing the "webSocketSession" helps alleviate the issue -but I am not very sure what is happening here. Do we need to handle threading issue in our handler implementation or is there a contention at AbstractHttpSockJsSession

I saw a similar issue @ #17911 but then it was specific to STOMP

---

Affects: 4.1.5

Issue Links:

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

[spring-projects-issues]

Rossen Stoyanchev commented

You're indeed expected to synchronize writes to the WebSocketSession. This is also the case for JSR-356 as explained in #17911 where there was a case of unsynchronized writing to the underlying WebSocket session.

When using the sub-protocol support (STOMP) we automatically take care of that by wrapping each session with ConcurrentWebSocketSessionDecorator (see SubProtocolWebSocketHandler). You could use that as well in the same fashion. When a new session is established, wrap it and then use the wrapper. More than synchronizing it also ensures that no one (slow) session can use more than one thread for sending at a time and also has timeout/buffer limits.

[spring-projects-issues]

Chandan commented

Thanks for reply. Do I need to upgrade to 4.1.8 -is there any fix for that? Or just using the ConcurrentWebSocketSessionDecorator is sufficient. I see here that its pretty straight forward http://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/web/socket/handler/ConcurrentWebSocketSessionDecorator.html, I need to wrap the websocket Object in this -

public ConcurrentWebSocketSessionDecorator(WebSocketSession delegate,
                                           int sendTimeLimit,
                                           int bufferSizeLimit)

sendTimeLimit -I guess this is maximum time to wait before message is discarded
bufferSizeLimit -is this per message? or A size to hold as many message as it can?

[spring-projects-issues]

Rossen Stoyanchev commented

sendTimeLimit and bufferSizeLimit are designed limit the impact of very slow clients (e.g. poor connection, mobile fading in/out). If any of the limits are violated the session is closed. The bufferSizeLimit is overall, not per message.

[spring-projects-issues]

Chandan commented

Thanks. The reference is silent about metric for int sendTimeLimit, int bufferSizeLimit -will it be sec/kb, min/mb .. not sure. Good, if the docs can be updated as well.