Clear subscriptions array to avoid memory leaks#4110
Closed
stalar wants to merge 2 commits intokatspaugh:mainfrom
Closed
Clear subscriptions array to avoid memory leaks#4110stalar wants to merge 2 commits intokatspaugh:mainfrom
stalar wants to merge 2 commits intokatspaugh:mainfrom
Conversation
Owner
|
Thanks for the PR! I'll need to investigate a bit more. |
Owner
|
I've made an improved PR based on this one: #4114 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Short description
I have observed memory leaks when using the minimap plugin.
In my application, I reuse the same wavesurfer instance and create a new MinimapPlugin each time I load new audio data. I also call destroy() on the old MinimapPlugin. Despite this, I see memory being leaked that was created from the minimap plugin.
Perhaps this reuse of an instance with different plugins is not an intended use-case in which case this PR is moot.
Implementation details
I found that the problem is with the following code:
When the plugin is destroyed, there is still a reference to it in the
subscriptionsarray.I could not find a good way to remove the entry in
subscriptionsso I opted for not adding it there at all. The drawback is thatunregister()on the event will not be called. This could lead to a plugin calling the destroy handler after the wavesurfer instance was destroyed, but that seems harmless.How to test it
minimap-leakexample and press the "Reload" button several timesChecklist