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(sharded): SSUBSCRIBE smessageBuffer memory leak (#528) #529

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

RoccoC
Copy link
Contributor

@RoccoC RoccoC commented Mar 1, 2024

This is a proposed fix for #528.

This pull request introduces a change to the sharded adapter's SSUBSCRIBE logic: Previously, for each dynamic channel/room, a unique listener was added to the client's smessageBuffer. This approach led to a large number of listeners (resulting in MaxListenersExceededWarning), especially in scenarios with many dynamic channels. Further, listeners were not being removed when unsubscribing, leading to a memory leak.

The new implementation replaces the multiple listeners with a single smessageBuffer listener. This listener is registered once and handles all dynamic channels by maintaining specific channel handlers in a Map. Listeners are added to this Map in SSUBSCRIBE and removed from the Map in SUNSUBSCRIBE.

Thanks for your review! 🙏

@darrachequesne darrachequesne merged commit 2113e8d into socketio:main Mar 13, 2024
2 checks passed
@darrachequesne
Copy link
Member

@RoccoC awesome, thanks 👍

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.

3 participants