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

Fix memory leak with multiple WebSocket servers on the same HTTP server #339

Merged
merged 1 commit into from
Dec 3, 2019
Merged

Conversation

nazar-pc
Copy link
Contributor

My use case involves several WebSocket servers on the same HTTP server that each care about specific path in URL (for instance, one handles ws://localhost/ws1 and another ws://localhost/ws2).
In this scenario I do not want to reject request if it doesn't correspond to particular WebSocketServer's path. However, if I do not accept and do not reject request, it will be kept in memory even after original socket connection is closed entirely.

One workaround is to use wsServer.handleRequestResolved(request);, which is not a public API and not intuitive.

I decided that at least when socket is closed, request should be removed from memory.

BTW, I'd also like to see request.ignore() method that allows to clean memory earlier (without explicitly accepting or rejecting the whole connection), but in either case, this PR is a last defense from memory leaks in mentioned and similar scenarios.

@nazar-pc
Copy link
Contributor Author

nazar-pc commented May 2, 2019

Can someone look at this, please?

@nazar-pc
Copy link
Contributor Author

Can someone finally review this?

@ibc
Copy link
Collaborator

ibc commented Jul 17, 2019

I'm afraid this project is not being maintained for a while. Trying to contact @theturtle32 to assign new maintainers with NPM permissions.

@ibc ibc merged commit e60f68d into theturtle32:master Dec 3, 2019
@nazar-pc nazar-pc deleted the patch-1 branch December 3, 2019 21:37
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

Successfully merging this pull request may close these issues.

2 participants