Fix memory leaks in Jam Recorder#1073
Merged
softins merged 5 commits intojamulussoftware:masterfrom Feb 22, 2021
Merged
Conversation
Contributor
|
I've compiled the updated code and put it live on Swedish Open Jam 1 and 2. Now just waiting to see what's happens. |
pljones
reviewed
Feb 20, 2021
pljones
reviewed
Feb 20, 2021
pljones
reviewed
Feb 20, 2021
pljones
reviewed
Feb 20, 2021
pljones
reviewed
Feb 20, 2021
hoffie
approved these changes
Feb 21, 2021
Member
There was a problem hiding this comment.
Looks good to me! I can confirm that the following leaks no longer appear in valgrind after this PR. I therefore suggest merging this and continue leak hunting afterwards. :)
==129778== by 0x1AD536: recorder::CJamClient::CJamClient(long long, int, QString, CHostAddress, QDir) (in /home/christian/Projects/jamulus/Jamulus)
==129778== by 0x1AEAB7: recorder::CJamSession::Frame(int, QString, CHostAddress, int, CVector<short>, int) (in /home/christian/Projects/jamulus/Jamulus)
==129778== by 0x1B18C6: recorder::CJamRecorder::OnFrame(int, QString, CHostAddress, int, CVector<short>) (in /home/christian/Projects/jamulus/Jamulus)
==129778== 438 (88 direct, 350 indirect) bytes in 1 blocks are definitely lost in loss record 120 of 165
==129778== at 0x483ADEF: operator new(unsigned long) (vg_replace_malloc.c:342)
==129778== by 0x5AF54E5: QObject::QObject(QObject*) (in /usr/lib/libQt5Core.so.5.15.2)
==129778== by 0x1ADF21: recorder::CJamSession::DisconnectClient(int) (in /home/christian/Projects/jamulus/Jamulus)
==129778== by 0x1AE1A8: recorder::CJamRecorder::OnDisconnected(int) (in /home/christian/Projects/jamulus/Jamulus)
==129778== 876 (112 direct, 764 indirect) bytes in 2 blocks are definitely lost in loss record 129 of 165
==129778== at 0x483ADEF: operator new(unsigned long) (vg_replace_malloc.c:342)
==129778== by 0x1ADF14: recorder::CJamSession::DisconnectClient(int) (in /home/christian/Projects/jamulus/Jamulus)
==129778== by 0x1AE1A8: recorder::CJamRecorder::OnDisconnected(int) (in /home/christian/Projects/jamulus/Jamulus)
==129778== by 0x5AEC581: QObject::event(QEvent*) (in /usr/lib/libQt5Core.so.5.15.2)
==129778== 1,744 (352 direct, 1,392 indirect) bytes in 4 blocks are definitely lost in loss record 146 of 165
==129778== at 0x483ADEF: operator new(unsigned long) (vg_replace_malloc.c:342)
==129778== by 0x5AF54E5: QObject::QObject(QObject*) (in /usr/lib/libQt5Core.so.5.15.2)
==129778== by 0x1ADF21: recorder::CJamSession::DisconnectClient(int) (in /home/christian/Projects/jamulus/Jamulus)
==129778== by 0x1AE1A8: recorder::CJamRecorder::OnDisconnected(int) (in /home/christian/Projects/jamulus/Jamulus)
pljones
reviewed
Feb 21, 2021
src/recorder/jamcontroller.cpp
Outdated
| { | ||
| if ( pJamRecorder != nullptr ) | ||
| { | ||
| // We have a recorder running already. Terminate it. |
Collaborator
There was a problem hiding this comment.
Just so I don't get so nervous reading this, can it be a clearer comment, please:
// We have a reference to a CJamRecorder instance that should now have finished. Clean up the instance.
(pJamRecorder->moveToThread ( pthJamRecorder ); puts pJamRecorder into the thread and the pthJamRecorder ->wait() above - if we had one - waited for it to finish.)
pljones
approved these changes
Feb 22, 2021
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
4 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.
Fixes #1019
EDIT: fixes leaks in the recorder, but there appear still to be others.