feat(Views): stop disposing #946
Merged
Conversation
This file contains 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.
This should fix all the random segfaults
Why were we disposing?
Ref cycles would prevent Views from being destroyed. They would drop from ~60 to ~50 refs. That would also prevent websockets from closing.
Disposing unrefs everything, allowing them to be freed from memory however that had the cost of... losing reference to them and their children sometimes.
Now what?
After some debugging, I pinned it down to
Gtk.ListBox#bind_model. We have to clear the binding when it's time to be disposed, but there's no appropriate stage to do that on as dispose or destroy won't ever be called because of it.Instead, manually remove the binding when it's time to be disposed, by using the same hack as before. The difference now is that we gracefully allow the view and its children to be destroyed, giving also the chance to notice and debug memory leaks.
Why keep the hack?
I believe the current choices are:
Gtk.ListBox#bind_modelI don't want to do (2), at least not right now. Exposing just the model and letting descendants to just add items to it, rather than overriding a bunch of methods.
* Another possibility would be to clear it when removing the pages, but... that's not always possible as we have to keep track of navigation view replacements and nested views