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

Resource leaks when using uWSGI WebSockets #296

Closed
juneoh opened this issue Nov 23, 2022 · 2 comments · Fixed by #301
Closed

Resource leaks when using uWSGI WebSockets #296

juneoh opened this issue Nov 23, 2022 · 2 comments · Fixed by #301
Assignees
Labels

Comments

@juneoh
Copy link
Contributor

juneoh commented Nov 23, 2022

IMPORTANT: If you have a question, or you are not sure if you have found a bug in this package, then you are in the wrong place. Hit back in your web browser, and then open a GitHub Discussion instead. Likewise, if you are unable to provide the information requested below, open a discussion to troubleshoot your issue.

Describe the bug
A WebSocket PONG miss while using uWSGI will leave an open TCP connection on python-engineio, gradually consuming more CPU.

To Reproduce

  1. Run a python-engineio based websocket server, e.g. using Flask-SocketIO
  2. Connect to the server with any websocket client.
  3. On the client side, intentionally miss a WebSocket PONG.
  4. uWSGI will complain that the pong has been missed, emitting a log (uWSGI code), resulting uwsgi_websocket_recv() to return an empty buffer.
  5. Upon the empty buffer, uWSGI's Python binding will raise an IOError during uwsgi.websocket_recv() (uWSGI code).
  6. python-engineio's uWSGIWebSocket.wait() catches the IOError and returns None (python-engineio code).
  7. python-engineio's Socket._websocket_handler() will receive the returned None, break out of the main loop (python-engineio code), join the writer_task(), and call Socket.close().
  8. Socket.close() will attempt to send the CLOSE packet, but since writer_task() is already joined, the packet will only be queued, never sent.
  9. Since uwsgi_disconnect() is never called in this flow, uWSGI's socket is never explicitly closed, hence leaked.

Expected behavior
The connection should be explicitly closed.

Additional context
Seems like #237 was about this. I'll see if I can come up with a patch, but I'll need some help for it not to affect other websocket drivers.

@miguelgrinberg miguelgrinberg self-assigned this Nov 24, 2022
@juneoh
Copy link
Contributor Author

juneoh commented Dec 6, 2022

Forgot the most important part: huge thank-you for your work!

As for the patch, I've added self.close() to all IOError catches in uWSGIWebSocket. So far it looks like the connections are now getting freed alright.

@juneoh
Copy link
Contributor Author

juneoh commented Dec 9, 2022

One more: it turns out that uWSGI can also raise IOError during send(). self._send(msg) within uWSGIWebSocket.wait() must also catch it for another self.close(). Otherwise, it can leave the file descriptor to the TCP port unbound, which seems to make select() for read always return immediately, and cause select_greenlet_runner() to go in an infinite loop, eating up CPU cycles.

And also, the close() method can be reordered by putting uwsgi disconnect to the back, to prevent select_greenlet_runner() raising OSError. Harmless error, but still.

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

Successfully merging a pull request may close this issue.

2 participants