Fix a memory leak
AbandonedPublic

Authored by jtamate on Dec 3 2017, 9:53 AM.

Details

Reviewers
None
Group Reviewers
Frameworks
Summary

setup was created but never released, now changed to a member pointer
to be deleted in the destructor.
According to valgrind, the leak is gone.

Test Plan

this leak is gone

25519== 58,922 (80 direct, 58,842 indirect) bytes in 1 blocks are definitely lost in loss record 2,937 of 2,937

25519== at 0x4C2E08F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)

25519== by 0xAC57C3C: QHashData::allocateNode(int) (in /usr/lib64/libQt5Core.so.5.9.2)

25519== by 0x82F3089: QHash<QString, KuitTag>::createNode(unsigned int, QString const&, KuitTag const&, QHashNode<QString, KuitTag>**) (qhash.h:551)

25519== by 0x82EF67F: QHash<QString, KuitTag>::operator[](QString const&) (qhash.h:751)

25519== by 0x82E10EA: KuitSetupPrivate::setTagPattern(QString const&, QStringList const&, Kuit::VisualFormat, KLocalizedString const&, QString (*)(QStringList const&, QString const&, QHash<QString, QString> const&, QString const&, QStringList const&, Kuit::VisualFormat), int) (kuitmarkup.cpp:546)

25519== by 0x82E7FA6: KuitSetupPrivate::setDefaultMarkup() (kuitmarkup.cpp:994)

25519== by 0x82E97E6: KuitSetup::KuitSetup(QByteArray const&) (kuitmarkup.cpp:1089)

25519== by 0x82E101C: Kuit::setupForDomain(QByteArray const&) (kuitmarkup.cpp:504)

25519== by 0x82E9979: KuitFormatterPrivate::format(QByteArray const&, QString const&, QString const&, Kuit::VisualFormat) const (kuitmarkup.cpp:1212)

25519== by 0x82ED234: KuitFormatter::format(QByteArray const&, QString const&, QString const&, Kuit::VisualFormat) const (kuitmarkup.cpp:1692)

25519== by 0x82C8F0B: KLocalizedStringPrivate::formatMarkup(QByteArray const&, QString const&, QString const&, QString const&, Kuit::VisualFormat) const (klocalizedstring.cpp:800)

25519== by 0x82C7EC2: KLocalizedStringPrivate::toString(QByteArray const&, QStringList const&, Kuit::VisualFormat, bool) const (klocalizedstring.cpp:610)

Diff Detail

Repository
R249 KI18n
Branch
memleak (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
jtamate created this revision.Dec 3 2017, 9:53 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 3 2017, 9:53 AM
jtamate requested review of this revision.Dec 3 2017, 9:53 AM
apol added a subscriber: apol.Dec 3 2017, 9:57 AM
apol added inline comments.
src/kuitmarkup.cpp
1213

That's really weird, are we deleting something that was returned to us by reference?

Looks like the issue is in setupForDomain...

As I understand the problem, the reference was created (as a pointer) in setupForDomain, but was stored in a local reference in format (line 1221).
As I'm not sure about the lifetime of the reference, I just delete it in the destructor.

aacid added a subscriber: aacid.Dec 3 2017, 11:44 AM

As I understand the problem, the reference was created (as a pointer) in setupForDomain, but was stored in a local reference in format (line 1221).
As I'm not sure about the lifetime of the reference, I just delete it in the destructor.

But it doesn't belong exclusively to you, no? So you can't delete it.

It should not deleted in KuitFormatterPrivate, it's in static data object. ~KuitStaticData should delete domainSetups.

mpyne added a subscriber: mpyne.Dec 3 2017, 4:52 PM

I think ~KuitStaticData() *does* delete domainSetups. But deleting that QHash doesn't delete the underlying values. That means the real leak is at "Kuit::setupForDomain(QByteArray const&) (kuitmarkup.cpp:504)".

If that's the case, then adding a destructor to the KuitStaticData class (along lines of ~KuitStaticData() { qDeleteAll(domainSetups); }) should also clear up the leak, and do so in a safer fashion without trampling on a shared static object.