981966 - ogg_stream_init leaking body_data

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Andrew McCreight [:mccr8]]

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.

Comment 1

[Nathan Froyd [:froydnj]]

Is this a new leak?  Or is this an old leak that hadn't been analyzed before OggReporter showed up?

Comment 2

[Andrew McCreight [:mccr8]]

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.

Comment 3

[Nathan Froyd [:froydnj]]

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.

Comment 4

[Andrew McCreight [:mccr8]]

Ah, right, my mistake.  So I guess this is just libogg leaking some memory.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: ogg stacks

Running LSAN locally, I got more complete stacks.  This is happening under mozilla::DOMMediaStream::CheckTracksAvailable.

Comment 6

[Nathan Froyd [:froydnj]]

Attachment: fix OggWriter memory leak

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?

Comment 7

[Andrew McCreight [:mccr8]]

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

Comment 8

[Nathan Froyd [:froydnj]]

Aha, one should be using ogg_stream_clear instead of ogg_stream_destroy.

Comment 9

[Nathan Froyd [:froydnj]]

Attachment: fix OggWriter memory leak

Better patch.  Andrew, WDYT about this?

Comment 10

[Andrew McCreight [:mccr8]]

Comment on attachment 8389358 [details] [diff] [review]
fix OggWriter memory leak

Yep, that gets rid of this leak.

Comment 11

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89b07f1c81ee

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/89b07f1c81ee