Opened 13 years ago

Closed 13 years ago

#1783 closed patch (fixed)

Memory Leak - StreamDataListener Handling

Reported by: Stuart Auchterlonie Owned by: danielk
Priority: minor Milestone: 0.20
Component: mythtv Version: head
Severity: medium Keywords: memory leak
Cc: Ticket locked: no

Description (last modified by danielk)

Found a memory leak in the stream data Listener handling in siparser.

Patch attached.

Attachments (5)

streamdata-memory-leak.diff (764 bytes) - added by Stuart Auchterlonie 13 years ago.
Patch to fix memory leak in streamdata listener handling
streamdata_leak_fix.2.diff (2.0 KB) - added by Janne <janne-mythtv@…> 13 years ago.
streamdata_leak_fix.3.diff (22.4 KB) - added by Janne <janne-mythtv@…> 13 years ago.
delete_all_streamdata.diff (643 bytes) - added by Janne <janne-mythtv@…> 13 years ago.
1783.patch (749 bytes) - added by danielk 13 years ago.
deletes the streamdata with the proper check

Download all attachments as: .zip

Change History (19)

Changed 13 years ago by Stuart Auchterlonie

Attachment: streamdata-memory-leak.diff added

Patch to fix memory leak in streamdata listener handling

comment:1 Changed 13 years ago by danielk

Description: modified (diff)
Type: patchdefect

This patch won't do since the SIParser is going away and this would segfault when both SIParser and DVBRecorder delete the same object.

But the bug is valid and I'll write a proper fix within the next few weeks if no one beats me to it.

The fix will either transfer ownership of the stream data to TVRec, or just delete the stream data when we teardown a DTVSignalMonitor with a stream data that a recorder does not reference.

Changed 13 years ago by Janne <janne-mythtv@…>

Attachment: streamdata_leak_fix.2.diff added

comment:2 Changed 13 years ago by Janne <janne-mythtv@…>

Type: defectpatch

streamdata_leak_fix.2.diff deletes the streamdata in DTVSignalmonitor if it has no further MPEGListeners.

This resolves most of the outstanding memory leaks.

comment:3 Changed 13 years ago by Janne <janne-mythtv@…>

the patch doesn't work. I'll update it.

Changed 13 years ago by Janne <janne-mythtv@…>

Attachment: streamdata_leak_fix.3.diff added

comment:4 Changed 13 years ago by Janne <janne-mythtv@…>

moved streamdata control to TVRec.

All recorders except DVBRecorder are untested. FirewireRecorderBase? and HDTVRecorder need a thorough review. I commented the creation of StreamData? in their constructors.

comment:5 Changed 13 years ago by danielk

Owner: changed from danielk to Stuart Auchterlonie

Changed 13 years ago by Janne <janne-mythtv@…>

Attachment: delete_all_streamdata.diff added

comment:6 Changed 13 years ago by Janne <janne-mythtv@…>

delete_all_streamdata.diff fixes the leak with a single delete in tv_rec.cpp

comment:7 Changed 13 years ago by Stuart Auchterlonie

(In [10188]) Refs #1783. Applies Janne's patch to plug memory leak in the streamdata handling.

Still needs to applied to head.

comment:8 Changed 13 years ago by Stuart Auchterlonie

Resolution: fixed
Status: newclosed

Closed by [10238]

comment:9 Changed 13 years ago by Stuart Auchterlonie

Resolution: fixed
Status: closedreopened

Re-opened by [10244]

comment:10 Changed 13 years ago by danielk

Component: dvbmythtv
Owner: changed from Stuart Auchterlonie to danielk
Status: reopenednew

comment:11 Changed 13 years ago by danielk

Type: patchdefect

Changed 13 years ago by danielk

Attachment: 1783.patch added

deletes the streamdata with the proper check

comment:12 Changed 13 years ago by danielk

Type: defectpatch

comment:13 Changed 13 years ago by Stuart Auchterlonie

I've had the backend running under valgrind for the last 4hrs, and it is quite happy with it when this patch is applied.

comment:14 Changed 13 years ago by danielk

Resolution: fixed
Status: newclosed

(In [10613]) Fixes #1783. Fixes the last stream data leak.

This is in the same piece of code from [10188] that caused segfaults and needed to be reverted in [10238]. I've added a check to make sure the MPEGStreamData is not still in use before deleting it. I've been running this for a while without any segfaults here, and Stuart A has confirmed that it fixes the leak with Valgrind.

Note: See TracTickets for help on using tickets.