Memory leak: Application views + screenshots never release memory to the system - #2437
Open
lewisgoddard wants to merge 2 commits into
Open
Memory leak: Application views + screenshots never release memory to the system#2437lewisgoddard wants to merge 2 commits into
lewisgoddard wants to merge 2 commits into
Conversation
…gleton set_branding() connected a lambda to the process-lifetime Granite.Settings singleton that captured its local `package` parameter. Capturing a local makes Vala emit g_signal_connect_data() with a heap closure block holding strong refs to both the Screenshot widget and the Package, instead of the g_signal_connect_object() it emits when only `this` is captured. Nothing ever disconnected, so every Screenshot ever displayed stayed alive for the life of the process along with its decoded image data. Connect in construct(), where no local is in scope to capture, and keep the colour in a field. This also fixes set_branding() stacking an additional permanent handler on each call.
…close The motion-controller enter/leave handlers and the scroll value-changed handler captured construct-locals, so Vala emitted g_signal_connect_data() with a strong reference back to the view. Because the view owns those child widgets, that closed a reference cycle: every AppInfoView (and all of its screenshots) stayed alive after being popped from the navigation view, so browsing leaked hundreds of MB that were never reclaimed. Promote the captured locals to fields so the handlers capture only `this` and Vala emits the weak g_signal_connect_object(), letting the view finalize on navigation. Also malloc_trim() once the window is destroyed so closing it returns the freed browsing memory to the OS.
Member
|
How did you find this solution? |
Member
Author
|
Only destroying the app views on close seemed to leave the memory allocated to the app, so I used |
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
2 participants
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.
Every AppInfoView (and all of its screenshots) stayed alive after being popped from the navigation view, so browsing leaked hundreds of MB that were never reclaimed. (I initially thought this was just a screenshot issue.)
Now AppInfoView is free'd within the app memory pool when navigating away, and closing the window releases all free'd memory to the system.
Before: Browing apps brought memory usage to 1.4GiB, not free'd on close.
After: Browsing apps brought memory usage to ~800MiB, free'd to ~500MiB on close.