Opened 11 years ago
Closed 10 years ago
#9945 closed bug (fixed)
BMediaFile leaks ca. 100 KB
Reported by: | ttcoder | Owned by: | nobody |
---|---|---|---|
Priority: | normal | Milestone: | R1 |
Component: | Audio & Video/Codecs | Version: | R1/Development |
Keywords: | ffmpeg | Cc: | |
Blocked By: | Blocking: | ||
Platform: | All |
Description
Calling new / SetTo() / delete (passing a valid audio file to SetTo()) results in circa 100 kilobytes heap memory leaked each time. Ten audio files loaded -> 1 MB memory leak and so on.
See simple test case attached.
Attachments (1)
Change History (17)
by , 11 years ago
Attachment: | BMediaFile-leak.cpp added |
---|
comment:1 by , 11 years ago
Might be ffmpeg that's leaking memory: inspecting the allocated 260 bytes blocks in Debugger shows "pmff" tags at the beginning of these blocks; and I sometime get the error message here going like
PluginManager: Error, unloading PlugIn ffmpeg with usecount 1
Tentatively setting priority to High. This is something that would need be adressed sooner rather than later since in my testing a radio station can run out of virtual memory in as little as 4~5 days with such a bad-ass leak :-o ; and this leak might not be hard to track down (unless it's deep in ffmpeg eh..?). Or at least I'd need a work-around to keep me going until it's fixed.
comment:2 by , 11 years ago
Component: | Kits/Media Kit → Audio & Video/Codecs |
---|---|
Owner: | changed from | to
comment:3 by , 11 years ago
The "pmff" tag seems to be the misc's ffmpeg media format we set by default in the global codec media formats table, gAVCodecFormats:
http://haiku.it.su.se:8180/source/xref/src/add-ons/media/plugins/ffmpeg/CodecTable.cpp#30
It's a 1024 entries table of media_format, which could be related to those ~100kb heap memory leak. Maybe somewhere we leak it somehow, in particular when ffmpeg plugin is unloaded while still somewhat used.
follow-up: 5 comment:4 by , 11 years ago
Philippe indeed, if the entries from that table are somehow copied to somewhere allocated with new or malloc() then that could be it. I see the 'ffmp' constant set at line 149.
Note to myself: if feeling like fishing in the dark later on, I could take a look at https://trac.ffmpeg.org/ticket/2937
I wonder if there is a way I could set a "watchpoint" in Debugger on a given heap area (rather than watch a given variable).. If yes then, knowing in advance one of the addresses of leaked memory blocks (from a previous run with printf() debug output ..etc) I could invoke debugger() at the beginning of main(), set the watchpoint on that address, click "Run" and get immediately notified (with an explicit backtrace) when that memory block gets malloc()ed. If I know who malloc()s it I'll have a good lead on who should be free()ing it yet fails to, probably.
EDIT: I mean to say that if somebody calls malloc(260) but does not write any contents at that address, then watching the address of that block won't trigger a debugger notification at malloc() time but rather at a later time when somebody (else) writes into that block, presumably. What I would need is to "watch" the malloc chain or something.
comment:5 by , 11 years ago
Replying to ttcoder:
I wonder if there is a way I could set a "watchpoint" in Debugger on a given heap area (rather than watch a given variable).. If yes then, knowing in advance one of the addresses of leaked memory blocks (from a previous run with printf() debug output ..etc) I could invoke debugger() at the beginning of main(), set the watchpoint on that address, click "Run" and get immediately notified (with an explicit backtrace) when that memory block gets malloc()ed. If I know who malloc()s it I'll have a good lead on who should be free()ing it yet fails to, probably.
You can set such a watchpoint ; when you right click -> watch, the resulting dialog just prepopulates the address of the variable, but you're free to change that to an arbitrary address. However, that won't do you any good here, Haiku uses address space layout randomization, so the addresses will not in any way be predictable between runs.
comment:7 by , 11 years ago
Priority: | high → normal |
---|
@korli: Roxors! kudos to you. The leak went down dramatically from 100KB (in multiple allocs) to 2072 bytes (in one alloc):
src_plugins> LD_PRELOAD=libroot_debug.so ./BMediaFile-leak (....) total allocations: 824; total bytes: 196008 total allocations: 826; total bytes: 200152 total allocations: 827; total bytes: 202224 total allocations: 828; total bytes: 204296 total allocations: 829; total bytes: 206368 total allocations: 830; total bytes: 208440 total allocations: 831; total bytes: 210512 total allocations: 832; total bytes: 212584 total allocations: 833; total bytes: 214656
Leaking 50 times more slowly than before, a station could go on for close to a year before even needing an app restart. Bug's 99% fixed and no longer urgent in my mind at all.
@anevilyak: no matter how many times I run the above .cpp from Terminal, its new BMediaFile
constructor call always allocates the object at the same address, as is the embedded mp3 codec (see below); I probably misunderstand what ASLR is about though, that would be odd if my simple test case had uncovered a kernel problem of any sort. This is with hrev46033 retrieved just now.
Run 1:
src_plugins> LD_PRELOAD=libroot_debug.so ./BMediaFile-leak [mp3 @ 0x18092800] max_analyze_duration 5000000 reached at 5015510 ...
Run 2:
src_plugins> LD_PRELOAD=libroot_debug.so ./BMediaFile-leak [mp3 @ 0x18092800] max_analyze_duration 5000000 reached at 5015510 ...
comment:9 by , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
The ctor leak is back, though much smaller now. After noticing ticket:9458#comment:2 I thought of running the test file here (including an augmented version that also calls TrackAt()) and we do get one leak when calling only the ctor, and an additional second leak when calling TrackAt() (so that's reading tracks, whereas #9458 is apparently about CreateTrack()):
~/Desktop/New folder> uname -a Haiku shredder 1 hrev48619 Jan 6 2015 08:42:39 BePC x86 Haiku ~/Desktop/New folder> gcc -g BMediaFile-leak.cpp -lbe -lmedia -lroot_debug ~/Desktop/New folder> LD_PRELOAD=libroot_debug.so ./BMediaFile-leak (..) total allocations: 1631; total bytes: 388946 total allocations: 1632; total bytes: 389142 total allocations: 1633; total bytes: 389338 total allocations: 1634; total bytes: 389534 total allocations: 1635; total bytes: 389730 ~/Desktop/New folder> # ctor only: leaks 1 alloc, 196 bytes ################################################### ~/Desktop/New folder> ~/Desktop/New folder> gcc -g BMediaFile-leak.cpp -lbe -lmedia -lroot_debug ~/Desktop/New folder> LD_PRELOAD=libroot_debug.so ./BMediaFile-leak (..) total allocations: 1637; total bytes: 390122 total allocations: 1639; total bytes: 390514 total allocations: 1641; total bytes: 390906 total allocations: 1643; total bytes: 391298 total allocations: 1645; total bytes: 391690 ~/Desktop/New folder> # ctor+TrackAt(): leaks (1+)1 alloc, (196+)196 bytes ##############################
Looking at the history of thread.c I see one recent change that (going on a limb here...) could explain the regression: the introduction of TLS. Is there an easy way to check the code ? Otherwise I'll try to download the closest revs I can find that are before and after hrev47198 and do a before-and-after test to confirm.
follow-up: 13 comment:12 by , 10 years ago
Reviewed my owned/Cc'ed list of tickets yet again, and this stands out as the only important issue that is not yet reviewed and is in R1 rather than R1/b1, maybe i should start clean (with the new editing privileges the way to set a milestone is with New Ticket, and more importantly, it seems justified to start clean for this ticket since the new 196 bytes leak only might be related to the 2072 bytes leak last seen here). Anyone wants to objects to my adding a new ticket, give a shout here today or tomorrow :-)
comment:13 by , 10 years ago
Replying to ttcoder:
(with the new editing privileges the way to set a milestone is with New Ticket)
No, if you think an existing ticket deserves to be in a different milestone, then you can ask for it to be changed, and that request will be suitably evaluated. The only thing that's different is that it isn't simply a free-for-all where every random person with an account gets to decide priorities.
comment:14 by , 10 years ago
However, your new issue may as well be a completely different one, and having to read the 10 comments to understand that several things were fixed, and then the issue is back, isn't very useful. I would prefer a new ticket for the new problem (mentionning this one as it may be related). There are a lot of other places where BMediaTrack could be leaking memory.
comment:15 by , 10 years ago
Thanks much, created #11752 , so this one can be closed for good I believe.
comment:16 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Change filepath and compile: the leak is reproduced in a dozen lines or so