-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Accepting back-pressure from slow websocket clients #1367
Labels
Comments
I may be off but I think this blog post may be worth reading: https://vorpus.org/blog/some-thoughts-on-asynchronous-api-design-in-a-post-asyncawait-world/ |
fafhrd91
pushed a commit
that referenced
this issue
Jan 31, 2017
should be fixed in master |
Thanks for pointing to the problem |
This was referenced Jun 24, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Long story short
If a websocket client is slow to read data from the network -- perhaps because of a network failure, or because of a malicious client -- then AFAICT there's no way for an aiohttp websocket server to detect this and slow down sending or drop that client. Instead, the server will just keep sending data and the data will queue up indefinitely inside the server. In some cases, such as a websocket server that sends out an ongoing stream of updates without reading responses (e.g.: a chat server or something like a live twitter feed), then this makes it possible to trivially trigger memory leaks on servers by sending them just a few packets.
Steps to reproduce
Point this client:
At this server:
But remember to hit control-C before the server runs your machine out of memory and it starts swapping to death.
Solution
The simplest solution would be to add an async
drain
method to websocket handlers, similar toasyncio.StreamWriter
. Probably a better solution would be to makesend_str
and friends async methods so that users didn't have to remember to calldrain
explicitly, but of course this has API compatibility issues.The text was updated successfully, but these errors were encountered: