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

AbstractWebSocketSession.sendMessage race condition [SPR-14138] #18710

Closed
spring-issuemaster opened this issue Apr 9, 2016 · 2 comments
Closed

AbstractWebSocketSession.sendMessage race condition [SPR-14138] #18710

spring-issuemaster opened this issue Apr 9, 2016 · 2 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 9, 2016

Karsten Sperling opened SPR-14138 and commented

AbstractWebSocketSession.sendMessage() checks if the connection is still open via isOpen() and throws an IllegalArgumentException if not. However this does not guarantee that the connection will still be open by the time the underlying WS implementation actually tries to write. This means the caller still has to catch / handle the relevant IOException anyway.

It is also impossible for the caller to avoid this exception as the remote side can close the connection at any time, so calling isOpen() before sendMessage() would suffer from exactly the same problem. The correct approach for this kind of situation is to simply attempt the IO operation and to let it fail naturally with an IOException in the underlying implementation if the connection has in fact been closed.

So I suggest to completely remove the Assert.isTrue(isOpen(), ...); check.


Affects: 4.2.5, 4.3 RC1

Referenced from: commits e67a274, aafd46a

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 14, 2016

Venil Noronha commented

Hi,

I've implemented a fix for this issue and have created a pull request under the branch 4.2.x. Please review and merge.

My CLA Number is 149720151120072904.

Thanks,
Venil Noronha

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 18, 2016

Rossen Stoyanchev commented

Thanks for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.