search

Proj-Ascension / Client

Client repository for Project Ascension, an open source game launcher.
https://projectascension.io/
MIT License
212 stars 37 forks source link

Memory Leak when creating AddGameWizard #112

Open lolzballs opened 9 years ago

lolzballs commented 9 years ago

The pages in AddGameWizard are not having a parent set, causing a memory leak. As well, the onAddGameClicked() in Library is not setting itself as a parent, and is creating a memory leak as well.

this cannot be set as a parent as it leads to inheritance of the Library stylesheet.

lolzballs commented 9 years ago

On further inspection, various widgets in AddGameWizard need parents as well.

elken commented 9 years ago

Noted, will amend in the refactor inbound. Muchas gracias, senor.

JimViebke commented 9 years ago

Just curious about design decisions, why aren't managed pointers such as unique_ptr<T> and shared_ptr<T> being used? I don't see the reason behind using raw pointers in 2015 unless one is working with legacy code. In terms of performance, compilers can optimize unique_ptrs to the same machine code as raw pointers.

If it's just a matter of somebody doing it, let me know.

elken commented 9 years ago

Because there's no need for ownership or sharing and the objects won't outlive the parent. Not relevant to the issue.

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Hal9007 commented 9 years ago

To elaborate on the response by @elken above, Qt by nature favors heap-allocation of local QObjects for a number of reasons, opting to manage memory hierarchically throughout the application when respective QObjects are parented to one another. In this issue, some parents were never set, hence the memory leakage.

The sheer amount of QObjects we're initializing programmatically now and in the future favors raw pointers as a form of brevity, though yes, in any other case we would be doing otherwise.

elken commented 9 years ago

To further elaborate, we also want raw pointers as arguments and as local variables for the speed and leanness of just an address, we don't need the overhead *_ptr types provide.

elken commented 8 years ago

Issue seems to be resolved, reopen if not.

lolzballs commented 8 years ago

The memory leak still occurs on both wizards: http://i.imgur.com/Jd5Ztm0.gifv