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

memory leak if compression is enabled, on payload bigger than 25kb (or less too, if is converted into string) #3595

Closed
1 of 2 tasks
shadowofsoul opened this issue May 5, 2020 · 1 comment

Comments

@shadowofsoul
Copy link

shadowofsoul commented May 5, 2020

You want to:

  • report a bug
  • request a feature

Current behaviour

If you pass a payload (string or binary) of more than 25kb, you get not only 1000x latency, but also an infinite memory leak. Looks like sender objects are not "dequequed".

Steps to reproduce (if the current behaviour is a bug)

screenshot-heap-snapshot

Here is the fiddle to reproduce the case, including memory heap snapshot. Just run server.js and then client.js with Node.js.

Expected behaviour

Not to have a memory leak

Setup

  • OS: Linux Ubuntu 19.10
  • browser: Server version, nodejs
  • socket.io version: 2.3.0
  • NodeJS: v12.15.0

Other information (e.g. stacktraces, related issues, suggestions how to fix)

I have been digging the engine.io & Websocket code. I know for a fact, that the dequeue function at sender.js is not called and that generates the memory leak. Like that function works on the premise of the _deflating prop, made me think that disabling compression would help (and it did). In the example i provide, can also be observed that passing a less than 25kb payload but as string, has the same result. Unfortunately, i don't have the time to dig more into an unknown codebase for now, if this is not fixed in the future, i'll look at it again in more detail to help as much as i can.

@darrachequesne
Copy link
Member

For future readers:

I think the memory leak was due to the fact that packets are added to the queue faster than what zlib can compress.

If you replace the delay here with a more sane value like 10 ms (instead of 0.3 ms), then the queue gets properly emptied.

Disabling compression (which is now the default) or increasing the concurrencyLimit (see ws options) should fix the issue.

Related: websockets/ws#1202

Please reopen if needed.

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