watermill icon indicating copy to clipboard operation
watermill copied to clipboard

Clean up resources in Go channel pub sub [Memory leak]

Open avlajcic-axilis opened this issue 2 years ago • 0 comments

Issue

Part of the issue reported here https://github.com/ThreeDotsLabs/watermill/issues/391 After testing, I've found same issue in Publish method. It uses LoadOrStore for fetching subLock. https://github.com/ThreeDotsLabs/watermill/blob/d20a6051456863fa147b9f9034b66500a6f0ffdf/pubsub/gochannel/pubsub.go#L96 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 https://github.com/ThreeDotsLabs/watermill/pull/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 of LoadOrStore 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 https://github.com/ThreeDotsLabs/watermill/pull/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.

avlajcic-axilis avatar Oct 30 '23 10:10 avlajcic-axilis