fix(channels): remove empty channel entries on unsubscribe to prevent memory leak#4868
fix(channels): remove empty channel entries on unsubscribe to prevent memory leak#4868lesnik512 wants to merge 6 commits into
Conversation
ChannelsPlugin.unsubscribe removed the subscriber and unsubscribed the backend but left the now-empty set and its key in self._channels. With dynamic per-entity channel names this grew unbounded for the process lifetime. Dynamically created channels are now deleted once their last subscriber leaves; constructor-declared channels are preserved. Closes litestar-org#4867 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4868 +/- ##
=======================================
Coverage 67.28% 67.29%
=======================================
Files 293 293
Lines 15226 15229 +3
Branches 1727 1728 +1
=======================================
+ Hits 10245 10248 +3
Misses 4834 4834
Partials 147 147 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Arbitrary channels are deleted from ``_channels`` once their last subscriber leaves. A second ``unsubscribe`` for the same channel then indexed the missing entry and raised ``KeyError``. Use ``dict.get`` with an empty-set default so the existing "not subscribed" branch handles it instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks! The code is fine, just a little nit: comments look a bit verbose, I think we can trim them since the code isn't complicated enough. What do you think? |
I've compacted and removed some comments, please take a loog |
Co-authored-by: Vitaly <128831423+Vitaly312@users.noreply.github.com>
Vitaly312
left a comment
There was a problem hiding this comment.
Thanks. CI failure is unrelated to the changes(looks like some flaky test)
Vitaly312
left a comment
There was a problem hiding this comment.
Approved (again?). Anyway, I don't have write access :)
Description
ChannelsPlugin._channelsgrows without bound when the plugin is used witharbitrary_channels_allowed=Trueand dynamic channel names (e.g. one channel per user),because
unsubscribe()removed the subscriber and unsubscribed the backend but never deletedthe now-empty
set()nor its key fromself._channels. For long-lived applications this is amemory leak that only a process restart clears.
This PR removes a dynamically created channel from
_channelsonce its last subscriber leaves.Channels declared in the constructor (
channels=[...]) are preserved, since they back thegenerated WebSocket route handlers and the startup backend subscription.
Reproduction (before this PR)
Changes
ChannelsPluginrecords constructor-declared channels and, inunsubscribe(), deletes achannel key once its subscriber set is empty only for dynamically created channels.
1000-cycle no-leak check. Existing unsubscribe tests use declared channels and are unaffected.
Notes
RedisChannelsStreamBackend.stream_events()keeps astream_idscursor map that exhibits thesame unbounded-growth pattern; happy to address it here in a follow-up commit or a separate PR —
let me know your preference.
Closes
Closes #4867
🤖 Generated with Claude Code
📚 Documentation preview 📚: https://litestar-org.github.io/litestar-docs-preview/4868