Clean up resources in Go channel pub sub [Memory leak] #407
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
Part of the issue reported here #391
After testing, I've found same issue in
Publish
method. It usesLoadOrStore
for fetchingsubLock
.watermill/pubsub/gochannel/pubsub.go
Line 96 in d20a605
That means if lock for specified topic doesn't exist it will create a new one. Because that data doesn't get removed, it means it will fill up map with new data even if there are no subscribers for this topic. If we use dynamic name of topics map size will continue to increase over time.
With fix in #396 data will only get removed if subscriber disconnects from specific topic. If no subscribers connect at any point, data will never get removed.
Fix
For
Publish
method, I've added condition to check if new lock has been loaded or added, and if it's added remove it at the end.There is an option to use
Load
instead ofLoadOrStore
and do early return here because if lock doesn't exist, it means there are no subscribers to send data to. I did't want to change much of the logic, but let me know if you think that is something that could be done.Only difference would be is that we would need to move handling of persistent message before that check.
For Subscribe method, fix is introduced in #396 which is included is this PR.
Tests
Since these fixes are related to internal fields, I've added new test file with the same package name so we can test those fields.
First test asserts that all data from subscriber maps has been removed after subscribers disconnect.
Second tests asserts that no data is left over in subscribers lock map after publishing messages. Lock for single subscriber should still exist in map.