Unity

Merge lp://qastaging/~thomir-deactivatedaccount/unity/fix-panel-memory-leaks into lp://qastaging/unity

  1. fix-panel-memory-leaks
  2. Merge into trunk
Proposed by Thomi Richards
Status: Merged
Approved by: Thomi Richards
Approved revision: no longer in the source branch.
Merged at revision: 2257
Proposed branch: lp://qastaging/~thomir-deactivatedaccount/unity/fix-panel-memory-leaks
Merge into: lp://qastaging/unity
Diff against target: 27 lines (+3/-3)
2 files modified
plugins/unityshell/src/PanelIndicatorEntryView.cpp (+2/-2)
plugins/unityshell/src/PanelIndicatorEntryView.h (+1/-1)
To merge this branch: bzr merge lp://qastaging/~thomir-deactivatedaccount/unity/fix-panel-memory-leaks
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
Tim Penhey (community) Approve
Review via email: mp+101298@code.qastaging.launchpad.net

Commit message

Fix memory leak in PanelIndicatorEntryView.

Description of the change

Problem: PanelIndicatorEntryView leaks it's entry_texture_ member variable. This happens every time the icons are refreshed (such as every time an overlay is shown or hidden), which quickly amounts to a very large memory leak.

Solution: Delete the texture entry before re-creating it.

Tests: No tests, but valgrind no longer finds the memory leak.

UNBLOCK

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

You shouldn't manually delete BaseTexture* objects as they are nux::Objects.

Instead define the member variable as:

nux::ObjectPtr<BaseTexture> entry_texture_;

Destructor line then isn't needed, nor the constructor.

The assignment needs to change to:
  entry_texture_ = texture_ptr_from_cairo_graphics(cg);

In fact, it looks like all that has to change is the header definition and the assignment.

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

Sorry, needs fixing.

review: Needs Fixing
Revision history for this message
Tim Penhey (thumper) wrote :

You have a leak here because of the way nux::ObjectPtrs work :-|

  texture_from_cairo_graphics(cg);

returns a new BaseTexture*. This object is initially owned, so putting it into an object pointer adds another reference.

Use texture_ptr_from_cairo_graphics(cg); This method returns a wrapped object using the nux::ObjectPtr.

review: Needs Fixing
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

FIxed.

Revision history for this message
Tim Penhey (thumper) :
review: Approve
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Cool, thanks for the fix, I was sure I did that, maybe I've missed to commit that.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.