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

Note on thread safety when using SockJSConnection.send() #102

Open
ssche opened this issue May 9, 2016 · 4 comments
Open

Note on thread safety when using SockJSConnection.send() #102

ssche opened this issue May 9, 2016 · 4 comments

Comments

@ssche
Copy link

ssche commented May 9, 2016

The examples are misleading in the sense that they are directly using SockJSConnection.send() method which adds messages into Tornado's IOStream send buffer (_write_buffer). If this happens on a separate thread, messages sent to the web socket can get corrupted, especially under heavy load (this method isn't thread-safe).

To avoid this, the messages should be handled by Tornado's ioloop (by adding a callback, etc). An example for the thread-safe implementation is how sockjs-tornado emits heartbeats.

A note in the documentation or a modified example would be nice as it could help people (like me) to save a lot of time.

@mrjoes
Copy link
Owner

mrjoes commented May 9, 2016

sockjs-tornado is single-threaded and assumes that everything is happening in a single thread. If you plan to use threads, then you will have to handle synchronization as well.

As far as I know, Tornado does not use threads on its own, but I was not following latest developments.

@ssche
Copy link
Author

ssche commented May 9, 2016

I know now ;-). Just a gentle note somewhere in the docs would be great (or perhaps there's already one that I didn't find).

@mrjoes
Copy link
Owner

mrjoes commented May 9, 2016

Good idea, I will add a word about it. In general, none of the Tornado libraries are thread safe and there was nice blog post why and how to work around it (by calling add_callback on the ioloop instance). But I can't find the link right now.

@ssche
Copy link
Author

ssche commented May 9, 2016

Thanks. That link would also be great in the docs. I started using Tornado because of sockjs and your library (which is great, btw) and learnt about the tread safety the hard way...

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

No branches or pull requests

2 participants