Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 23 months ago

Closed 22 months ago

#10807 closed Patch - Bug Fix (Invalid)

[PATCH] libmythui: Prevent memory leak and dangling reference to MythUIType children

Reported by: Lawrence Rust <lvr@…> Owned by: stuartm
Priority: minor Milestone: unknown
Component: MythTV - General Version: Master Head
Severity: medium Keywords: MythUIType memory leak
Cc: Ticket locked: no

Description

Although ownership of all MythUIType objects is transferred to the parent during construction, the MythUIType destructor doesn't delete its children, resulting in a memory leak found by valgrind.

Also, because of the ownership semantics, MythUIType's dtor should be protected. This uncovers a bug in MythUIButtonList::CopyFrom? which should use the base DeleteChild? (rather than a direct delete) otherwise a dangling reference is left.

This bug and patch apply to git master, fixes/0.25 and fixes/0.24 (maybe more).

P.S. IMHO the base class XMLParseBase of MythUIType should be protected otherwise it defeats abstraction.

Attachments (1)

0001-libmythui-Prevent-memory-leak-of-MythUIType-children.patch (2.6 KB) - added by Lawrence Rust <lvr@…> 23 months ago.

Download all attachments as: .zip

Change History (7)

Changed 23 months ago by Lawrence Rust <lvr@…>

comment:1 Changed 23 months ago by stuartm

Children of a QObject are deleted by the QObject destructor thus shouldn't be leaking.

comment:2 Changed 23 months ago by Lawrence Rust <lvr@…>

==9163== 96 bytes in 6 blocks are possibly lost in loss record 1,446 of 2,430
==9163==    at 0x4026864: malloc (vg_replace_malloc.c:236)
==9163==    by 0x7B6765C: qMalloc(unsigned int) (qmalloc.cpp:55)
==9163==    by 0x7B99611: QMapData::node_create(QMapData::Node**, int, int) (qmap.cpp:140)
==9163==    by 0x5857220: QMap<int, MythUIType*>::node_create(QMapData*, QMapData::Node**, int const&, MythUIType* const&) (qmap.h:449)
==9163==    by 0x586E1B8: QMap<int, MythUIType*>::operator[](int const&) (qmap.h:530)
==9163==    by 0x586CD9D: MythUIStateType::AddObject(MythUIStateType::StateType, MythUIType*) (mythuistatetype.cpp:78)
==9163==    by 0x586DCAC: MythUIStateType::CopyFrom(MythUIType*) (mythuistatetype.cpp:286)
==9163==    by 0x586DDC1: MythUIStateType::CreateCopy(MythUIType*) (mythuistatetype.cpp:294)
==9163==    by 0x5855252: MythUIType::CopyFrom(MythUIType*) (mythuitype.cpp:838)
==9163==    by 0x589ED4C: MythUIGroup::CopyFrom(MythUIType*) (mythuigroup.cpp:24)
==9163==    by 0x5835D38: XMLParseBase::ParseUIType(QString const&, QDomElement&, QString const&, MythUIType*, MythScreenType*, bool) (xmlparsebase.cpp:499)
==9163==    by 0x586D7E1: MythUIStateType::ParseElement(QString const&, QDomElement&, bool) (mythuistatetype.cpp:229)
==9163== 
==9163== 96 bytes in 6 blocks are possibly lost in loss record 1,447 of 2,430
==9163==    at 0x4026864: malloc (vg_replace_malloc.c:236)
==9163==    by 0x7B6765C: qMalloc(unsigned int) (qmalloc.cpp:55)
==9163==    by 0x7B99611: QMapData::node_create(QMapData::Node**, int, int) (qmap.cpp:140)
==9163==    by 0x586E6D5: QMap<QString, MythUIType*>::node_create(QMapData*, QMapData::Node**, QString const&, MythUIType* const&) (qmap.h:449)
==9163==    by 0x586E0F8: QMap<QString, MythUIType*>::operator[](QString const&) (qmap.h:530)
==9163==    by 0x586C9A9: MythUIStateType::AddObject(QString const&, MythUIType*) (mythuistatetype.cpp:46)
==9163==    by 0x586DBAF: MythUIStateType::CopyFrom(MythUIType*) (mythuistatetype.cpp:275)
==9163==    by 0x586DDC1: MythUIStateType::CreateCopy(MythUIType*) (mythuistatetype.cpp:294)
==9163==    by 0x5855252: MythUIType::CopyFrom(MythUIType*) (mythuitype.cpp:838)
==9163==    by 0x58973FD: MythUIButtonList::CopyFrom(MythUIType*) (mythuibuttonlist.cpp:2431)
==9163==    by 0x588A816: MythUISpinBox::CopyFrom(MythUIType*) (mythuispinbox.cpp:160)
==9163==    by 0x5835D38: XMLParseBase::ParseUIType(QString const&, QDomElement&, QString const&, MythUIType*, MythScreenType*, bool) (xmlparsebase.cpp:499)

comment:3 Changed 23 months ago by Lawrence Rust <lvr@…>

In c9497cb775f3dd77829bf426bbdff61074ee8819/mythtv:

Fix incorrect deletion of a MythUIType object in MythUIButtonList::CopyFrom?() which left an invalid pointer in the list of children. Refs #10807

Signed-off-by: Stuart Morgan <smorgan@…>

comment:4 Changed 23 months ago by Lawrence Rust <lvr@…>

In 36364d1ef37a1478e495d1d79154609e851de242/mythtv:

Protect the MythUIType destructor to prevent it being called incorrectly. Refs #10807

Signed-off-by: Stuart Morgan <smorgan@…>

comment:5 Changed 22 months ago by beirdo

  • Owner set to stuartm
  • Status changed from new to assigned

comment:6 Changed 22 months ago by stuartm

  • Resolution set to Invalid
  • Status changed from assigned to closed

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.