Closed Bug 981966 Opened 6 years ago Closed 6 years ago

ogg_stream_init leaking body_data

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: mccr8, Assigned: froydnj)

References

(Blocks 1 open bug)

Details

(Keywords: memory-leak, Whiteboard: [lsan][MemShrink])

Attachments

(2 files, 1 obsolete file)

Quis custodiet ipsos custodes?

LSAN reports a number of OggReporter leaks in M1 that look like the following:

  Direct leak of 147456 byte(s) in 9 object(s) allocated from:
      #0 0x446395 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
      #1 0x7f4cee97b68a in OggReporter::Alloc(unsigned long) /builds/slave/try-l64-asan-00000000000000000/build/xpcom/build/nsXPComInit.cpp:419
      #2 0x7f4cf4be041c in ogg_stream_init /builds/slave/try-l64-asan-00000000000000000/build/media/libogg/src/ogg_framing.c:194
      #3 0x7f4cf2969399 in Init /builds/slave/try-l64-asan-00000000000000000/build/content/media/ogg/OggWriter.cpp:34
      #4 0x7f4cf2969399 in mozilla::OggWriter::OggWriter() /builds/slave/try-l64-asan-00000000000000000/build/content/media/ogg/OggWriter.cpp:20
      #5 0x7f4cf2931a87 in mozilla::MediaEncoder::CreateEncoder(nsAString_internal const&, unsigned char) /builds/slave/try-l64-asan-00000000000000000/build/content/media/encoder/MediaEncoder.cpp:131

It is a few hundred KB in total.
Is this a new leak?  Or is this an old leak that hadn't been analyzed before OggReporter showed up?
I've only started running LSAN recently, if that's what you mean.  I didn't notice it at first because I only recently started turning on frame pointers, and before that it was showing up with this useless stack:

Direct leak of 114688 byte(s) in 7 object(s) allocated from:
    #0 0x446395 in malloc /whatever/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7fdd069c60a8 in ogg_stream_init /somepath/media/libogg/src/ogg_framing.c:194

As for how we could be leaking an ISupports class without any indication in the XPCOM leak checker, see bug 982072 for a theory.
This stack doesn't show us leaking OggReporter.  It shows allocations done by OggReporter that are leaking.  OggReporter is just a wrapper class around malloc/calloc/realloc/free so that we can identify what libogg is allocating and keep track of it.
Ah, right, my mistake.  So I guess this is just libogg leaking some memory.
Summary: OggReporter leaks until shutdown → ogg_stream_init leaking body_data
Attached file ogg stacks
Running LSAN locally, I got more complete stacks.  This is happening under mozilla::DOMMediaStream::CheckTracksAvailable.
Attached patch fix OggWriter memory leak (obsolete) — Splinter Review
This patch is the minimum necessary; currently running through Try to see
whether it works correctly...Andrew, can you run it through your setup to see
if it fixes the leak?
Attachment #8389317 - Flags: feedback?(continuation)
Comment on attachment 8389317 [details] [diff] [review]
fix OggWriter memory leak

That gives me this:

==6828==ERROR: AddressSanitizer: attempting to call malloc_usable_size() for pointer which is not owned: 0x615000418018
    #0 0x441225 in malloc (/home/amccreight/ff-dbg-asan/dist/bin/firefox+0x441225)
    #1 0x7f4acf42fddd in malloc_usable_size (/home/amccreight/ff-dbg-asan/dist/bin/libxul.so+0x132addd)
    #2 0x7f4ad355fe5e in OggReporter::Free(void*) /home/amccreight/mc/xpcom/build/nsXPComInit.cpp:446
    #3 0x7f4ad1e5129e in ogg_stream_destroy /home/amccreight/mc/media/libogg/src/ogg_framing.c:231
    #4 0x7f4ad1e5123d in mozilla::OggWriter::~OggWriter() /home/amccreight/mc/content/media/ogg/OggWriter.cpp:28
    #5 0x7f4ad1e356ad in mozilla::OggWriter::~OggWriter() /home/amccreight/mc/content/media/ogg/OggWriter.cpp:26
    #6 0x7f4ad007cd7b in mozilla::MediaEncoder::~MediaEncoder() /home/amccreight/mc/content/media/encoder/MediaEncoder.h:73
    #7 0x7f4ad1dfc79b in mozilla::MediaStreamListener::Release() /home/amccreight/ff-dbg-asan/content/media/webspeech/recognition/../../../../dist/include/MediaStreamGraph.h:95
    #8 0x7f4ad1dfc811 in mozilla::MediaStream::RemoveAllListenersImpl() /home/amccreight/mc/content/media/MediaStreamGraph.cpp:1783
    #9 0x7f4ad1dfe1b5 in mozilla::MediaStream::DestroyImpl() /home/amccreight/mc/content/media/MediaStreamGraph.cpp:1790
    #10 0x7f4ad1e080b9 in mozilla::ProcessedMediaStream::DestroyImpl() /home/amccreight/mc/content/media/MediaStreamGraph.cpp:2437
    #11 0x7f4ad1dfa1a5 in mozilla::MediaStream::Destroy()::Message::Run() /home/amccreight/mc/content/media/MediaStreamGraph.cpp:1812
    #12 0x7f4ad1e08592 in mozilla::MediaStreamGraphImpl::RunThread() /home/amccreight/mc/content/media/MediaStreamGraph.cpp:1182
    #13 0x7f4acf506145 in mozilla::(anonymous namespace)::MediaStreamGraphInitThreadRunnable::Run() /home/amccreight/mc/content/media/MediaStreamGraph.cpp:1397
Attachment #8389317 - Flags: feedback?(continuation) → feedback-
Aha, one should be using ogg_stream_clear instead of ogg_stream_destroy.
Better patch.  Andrew, WDYT about this?
Attachment #8389317 - Attachment is obsolete: true
Attachment #8389358 - Flags: feedback?(continuation)
Comment on attachment 8389358 [details] [diff] [review]
fix OggWriter memory leak

Yep, that gets rid of this leak.
Attachment #8389358 - Flags: feedback?(continuation) → feedback+
Attachment #8389358 - Flags: review?(kinetik)
Attachment #8389358 - Flags: review?(kinetik) → review+
Assignee: nobody → nfroyd
https://hg.mozilla.org/mozilla-central/rev/89b07f1c81ee
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.