Opened 12 years ago

Closed 12 years ago

#3692 closed defect (fixed)

Memory leak in DVB DummyVideo code

Reported by: anonymous Owned by: danielk
Priority: minor Milestone: 0.21
Component: dvb Version: head
Severity: medium Keywords:
Cc: Stuart Auchterlonie Ticket locked: no

Description

I originally posted this in #3317 but that ticket got closed without resolving my issue. For well over 6 months now, I've noticed that the backend slowly increases its virtual memory footprint. Which eventually leads to slowing the entire machine down (more-so on machines with small amounts of RAM and not enough swap.

Digging into it with pmap, I found lots of these:

pmap -d `pidof mythbackend`
...
a10de000       4 ----- 00000000a10de000 000:00000   [ anon ]
a10df000    8188 rwx-- 00000000a10df000 000:00000   [ anon ]
...

(That's 8MB of anonymous memory for those playing along). And I noticed that the number of these went up everytime I changed a channel.

An 8MB Anonymous page is just what you see when libpthread allocates a new stack when creating a thread, but after several channel changes, I was seeing lots more of these anonymous-memory entries than running threads. So I surmized that we weren't reclaiming the stack when a thread terminated.

I wrote a utility (included) to track all thread creation and what memory it was associated with (just a wrapper around libpthread) and was able to use it to map pthread_create calls to stack allocations.

Using this, I determined that each time dvbrecorder.cpp::StartDummyVideo? was called, the number of anonymous memory entries would increase, and these were never being reclaimed.

Simply adding a:

pthread_detach(_video_thread);

right after the pthread_create call (this could alternatively be done using attributes passed to pthread_create I suppose) allows the pthread stacks to be reclaimed once the thread terminates, and now myth no longer increases it's virtual memory footprint by 8MB every time a channel-change occurs.

Attachments (6)

pthread_leak.patch (490 bytes) - added by anonymous 12 years ago.
Patch to fix the issue
pmap.mythtv.pre (16.1 KB) - added by anonymous 12 years ago.
results of pmap after ~12 channel changes before the patch
pmap.mythtv.post (14.8 KB) - added by anonymous 12 years ago.
results of pmap after ~12 channel changes after the patch
pthreaddbg.c (21.8 KB) - added by anonymous 12 years ago.
library wrapper to keep track of stack-allocation in pthreads
3692-v1.patch (380 bytes) - added by danielk 12 years ago.
Potential fix
3692-v1-alt1.patch (2.5 KB) - added by danielk 12 years ago.
Alternate potential fix

Download all attachments as: .zip

Change History (14)

Changed 12 years ago by anonymous

Attachment: pthread_leak.patch added

Patch to fix the issue

Changed 12 years ago by anonymous

Attachment: pmap.mythtv.pre added

results of pmap after ~12 channel changes before the patch

Changed 12 years ago by anonymous

Attachment: pmap.mythtv.post added

results of pmap after ~12 channel changes after the patch

Changed 12 years ago by anonymous

Attachment: pthreaddbg.c added

library wrapper to keep track of stack-allocation in pthreads

comment:1 Changed 12 years ago by anonymous

I should mention that this affects 0.20-stable as well as SVN head. And that there are probably a couple other places like this, this is just the most frequent (and thus most annoying) one.

comment:2 Changed 12 years ago by Stuart Auchterlonie

I looked into this as part of #3317.

What i found is that the code already does a pthread_join when the thread finishes. A pthread_join is supposed to do the same cleanup of a thread as would the completion of a detached thread.

Stuart

Changed 12 years ago by danielk

Attachment: 3692-v1.patch added

Potential fix

Changed 12 years ago by danielk

Attachment: 3692-v1-alt1.patch added

Alternate potential fix

comment:3 Changed 12 years ago by danielk

I've attached two potential fixes, the first '3692-v1.patch' is very simple, but I'm not terribly confident it will work. The second should work, it makes the dummy video thread starting and stopping more like other joined MythTV threads, but is a little more involved.

comment:4 Changed 12 years ago by anonymous

Neither one of these patches fixes the issue.

I added some debugging code and found that while StopDummyVideo?() gets called, we never get into the while() loop that calls pthread_join() This is because of this:

Index: dvbrecorder.cpp
===================================================================
--- libs/libmythtv/dvbrecorder.cpp     (revision 13823)
+++ libs/libmythtv/dvbrecorder.cpp     (working copy)
@@ -1137,7 +1137,7 @@
     }
 
     if (StreamID::IsVideo(info->streamType))
-        _stop_dummy = true;
+        StopDummyVideo();
 
     if (StreamID::IsAudio(info->streamType))
         GetTimeStamp(tspacket);

Which when added (without any of your patches applied) fixes the problem.

comment:5 Changed 12 years ago by anonymous

Well, I've done some testing and this seems to slow down channel changing in a way that my 1st post does not. It is spending ~ 0.5 sec or so waiting for the pthread_join to complete. Using the 'pthread_detach' method does not show this lag.

comment:6 Changed 12 years ago by Stuart Auchterlonie

Cc: Stuart Auchterlonie added

I'm tempted to make the dummy video thread a detached thread, as I don't believe we really need to wait around for the result of the thread. It could quite happily be left to die off all by itself.

comment:7 Changed 12 years ago by Stuart Auchterlonie

Milestone: unknown0.21
Version: unknownhead

comment:8 Changed 12 years ago by danielk

Resolution: fixed
Status: newclosed

(In [14438]) Fixes #3692. Fixes a memory leak in DVB Radio support in trunk.

Note: See TracTickets for help on using tickets.