limitExceeded is never reset in ConcurrentWebSocketSessionDecorator [SPR-17140]

[spring-projects-issues]

HenryOrz opened SPR-17140 and commented

In a spring-websocket application, Once org.springframework.web.socket.handler.ConcurrentWebSocketSessionDecorator bufferSizeLimit was exceeded, the method sendMessage() does't work anymore.

There is a limitExceeded flag in ConcurrentWebSocketSessionDecorator, when the bufferSizeLimit was exceeded, the flag will set to true, but it's never reset to false. The method tryFlushMessageBuffer() which called by sendMessage() will check the limitExceeded flag, so the sendMessage() does't work after bufferSizeLimit  was exceeded

Thanks Artem Bilan for answer my question on stackoverflow

---

Affects: 4.3.18, 5.0.8

Reference URL: https://stackoverflow.com/questions/51723413/how-to-flush-buffer-when-concurrentwebsocketsessiondecorator-buffersizelimit-exc

Issue Links:

• Prevent WebSocket buffer overflow through application-level flow control [SPR-16089] #20638 Prevent WebSocket buffer overflow through application-level flow control

Referenced from: commits 309ffc6

[spring-projects-issues]

Rossen Stoyanchev commented

ConcurrentWebSocketSessionDecorator puts an upper limit on the send time and the send buffer size. Once those limits are exceeded, a SessionLimitExceededException is raised and the session is terminated. That's the point of those limits, to set a threshold. So this is by design, it's not a bug.

[spring-projects-issues]

HenryOrz commented

@Rossen Stoyanchev Thank you! You are right, I have read the comment of the class, It's not a bug, I misunderstand it.

But I'm puzzled by this feature, if buffer limit was exceeded, and I have also caught the SessionLimitExceededException, but I still want to send message, how can I restore it?

I have tried to new ConcurrentWebSocketSessionDecorator to replace the exceeded one, but there is no public access to get the buffer, which means I lost the message in the buffer of old one. 

Thanks to @Juergen Hoeller for linking issue #20638, It's reminds me to do some basic check by method getBufferSize(). But if I just check it simply(just like the code below). It may bring some problem like race condition, the code will be complex if I want to make it correct

 

if((decorator.getBufferSize() + messageSize) < decorator.getBufferSizeLimit()){
   decorator.sendMessage();
}

Is there any way to send message more gracefully after limits exceeded?

 

Might we have a method to flush buffer and reset the limitExceed flag when limits exceeded?Just like

try {
   decorator.sendMessage();
} catch (SessionLimitExceededException e) {
   decorator.flushAndReset();
}

 Or could we remove the limitExceed condition in method tryFlushMessageBuffer() , and reset the limitExceed flag after all the message in buffer was flushed?

[spring-projects-issues]

Rossen Stoyanchev commented

There are only so many ways to deal with overflow. Currently it simply drops the connection once the limit is reached. We could add an option to enable an alternative overflow strategy such as to drop messages. Is that what you're looking for?

[spring-projects-issues]

HenryOrz commented

Thank you! That is what I'm looking for!

More precisely, I want a strategy to flush buffer when buffer overflow.

I have commit a pull request  on github. 

[spring-projects-issues]

Rossen Stoyanchev commented

I've implemented this a little differently. Basically it's controlled through an OverflowStrategy enum flag on the constructor and automatically applied when the limit is reached.

[spring-projects-issues]

HenryOrz commented

Thank you!

[KeithWoods]

We've hit a similar/same issue, with the ConcurrentWebSocketSessionDecorator in respect of the SessionLimitExceededException

I don't believe any code in ConcurrentWebSocketSessionDecorator actually closes the session when SessionLimitExceededException is thrown and when the strategy is OverflowStrategy.TERMINATE.

If you're using stomp on top of websockets that does handle this differently. For example in the SubProtocolWebSocketHandlerit catches and closes the session That logic seems to be missing from ConcurrentWebSocketSessionDecorator, it get left in an odd state, the app think it's ok but it won't send due to limitExceeded. The workaround is to catch the above exception when you're calling sendMessage() and close the session yourself.

I noticed in the above stackover post Thanks Artem Bilan mentioned it looks like a bug

[rstoyanchev]

The decorator is intended to raise an exception only and leave it to the surrounding code to take further action as necessary. There is also an ExceptionWebSocketHandlerDecorator that will close the session for any Exception that bubbles up.

As for the referenced SO discussion, that seems about doing the opposite, which is to reset limitExceeded flag and presumably continue using the session.

[PatrickFriedhoff]

I came across the same issue as @KeithWoods described.
As you say @rstoyanchev , if the decorator is only intended to throw an exception and should not close the session itself, then it would be great to update the javadoc. It still says:

"...At that time, the specified buffer-size limit and send-time limit will be checked and the session will be closed if the limits are exceeded."

We were relying on that documentation and ran into issues, because the session was not closed as documented.