Fixes memory leak of KItemListHeader
ClosedPublic

Authored by hallas on Jul 29 2018, 10:12 AM.

Details

Summary

Fixes memory leak of KItemListHeader
The KItemListHeader passed the listView parent object to the QObject
base class, but that pointer seems to always be nullptr causing the
KItemListHeader to not be memory managed by anything. Instead simple
use the listView as parent pointer.

Test Plan

This leak was found using Address Sanitizer

Diff Detail

Repository
R318 Dolphin
Branch
fixes_leak_of_item_list_header (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8780
Build 8798: arc lint + arc unit
hallas created this revision.Jul 29 2018, 10:12 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptJul 29 2018, 10:12 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
hallas requested review of this revision.Jul 29 2018, 10:12 AM

But why doesn't the KItemListView have a parent? Who deletes listView then?

But why doesn't the KItemListView have a parent? Who deletes listView then?

I don't know :) I can just see from Address Sanitizer that the KItemListView was not being freed, and I traced this to the fact that the parent pointer is nullptr. Could this be caused by other code changes previously? So this is essentially a leftover from previous behavior?

But why doesn't the KItemListView have a parent? Who deletes listView then?

Hi @elvisangelaccio - I have been digging a bit more into this, and the reason why the parent is nullptr at this point is because it is setup at a later point. Actually it ends up being set be KItemListController::setView function, but that only happens after the KItemListView (which is the class allocating the KItemListHeader instance) has been constructed. It essentially a Chicken-Egg problem, if you look at the constructor for DolphinView, you can see that the view is constructed before the controller, and it is actually the controller that ends up being the parent for the view. So therefore I still think this is the way to fix this small memory leak.

elvisangelaccio accepted this revision.Sat, Feb 23, 5:18 PM

But why doesn't the KItemListView have a parent? Who deletes listView then?

Hi @elvisangelaccio - I have been digging a bit more into this, and the reason why the parent is nullptr at this point is because it is setup at a later point. Actually it ends up being set be KItemListController::setView function, but that only happens after the KItemListView (which is the class allocating the KItemListHeader instance) has been constructed. It essentially a Chicken-Egg problem, if you look at the constructor for DolphinView, you can see that the view is constructed before the controller, and it is actually the controller that ends up being the parent for the view. So therefore I still think this is the way to fix this small memory leak.

Thanks for the investigation. Please put this information in the commit message, then push it :)

This revision is now accepted and ready to land.Sat, Feb 23, 5:18 PM
hallas updated this revision to Diff 52462.Sun, Feb 24, 5:22 PM

Updated commit message and rebase

hallas closed this revision.Sun, Feb 24, 5:23 PM