Add missing trainprogram destructor to fix resource leaks - #4573
Closed
Qhilm wants to merge 2 commits into
Closed
Conversation
The trainprogram class was missing a destructor, causing memory leaks: - zwift_auth_token (allocated at line 38 with new) - zwift_world (allocated at line 635 with new) - h (lockscreen pointer, allocated at line 642 with new) These are heap-allocated resources that were never freed, leading to memory leaks on every trainprogram object destruction. Fix: Add destructor that properly deletes these members before object destruction. The pelotonOCRsocket is already properly parented to the trainprogram object via new QUdpSocket(this), so Qt's parent-child mechanism will clean it up.
Owner
|
This is true only when you create a new program or loading a new one. It should be negligible for regular session. What do you think? I'm saying so because it's always hard to test if a patch is fine or not even if it seems safe |
Contributor
Author
|
I agree with you on both points: It's a very rare memory leak and it's impossible to properly test. Up to you. I like tidy code, but I have a slight OCD ;) |
Owner
|
Me too :) but I learned that 2 minutes of easy work can value a lot of headaches in the future 😂 |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
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
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.
Problem
The
trainprogramclass was missing a destructor, causing memory leaks:zwift_auth_token(allocated at line 38 withnew)zwift_world(allocated at line 635 withnew)h(lockscreen pointer, allocated at line 642 withnew)These are heap-allocated resources that were never freed, leading to memory leaks on every
trainprogramobject destruction.Root Cause
The
trainprogramclass follows the RAII pattern but was missing the destructor to clean up heap-allocated resources.Solution
Add a destructor that properly deletes the heap-allocated members:
delete zwift_auth_token;delete zwift_world;delete h;The
pelotonOCRsocketis already properly parented to thetrainprogramobject vianew QUdpSocket(this), so Qt's parent-child mechanism will clean it up automatically.Changes
src/trainprogram.h- Add destructor declaration~trainprogram();src/trainprogram.cpp- Add destructor implementation with properdeletestatements