1091704 - crash in _VEC_memcpy | mozilla::CircularByteBuffer::AppendElements(unsigned char const*, unsigned int)

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Liz Henry (:lizzard) (relman/hg->git project)]

This bug was filed from the Socorro interface and is 
report bp-c85ff40b-00e1-4892-abf6-003b92141029.
=============================================================

This is the #6 topcrash for Firefox 34, with 2055/174701 crashes in the last 7 days.  Most of these crashes were in 34.0b3.


Crashing thread:

0 	msvcr100.dll 	_VEC_memcpy 	
1 	xul.dll 	mozilla::CircularByteBuffer::AppendElements(unsigned char const*, unsigned int) 	content/media/AudioStream.h
2 	xul.dll 	mozilla::AudioStream::Write(float const*, unsigned int, mozilla::TimeStamp*) 	content/media/AudioStream.cpp
3 	xul.dll 	mozilla::AudioSink::PlayFromAudioQueue() 	content/media/AudioSink.cpp
4 	xul.dll 	mozilla::AudioSink::AudioLoop() 	content/media/AudioSink.cpp
5 	xul.dll 	nsRunnableMethodImpl<void ( nsScriptLoader::*)(void), void, 1>::Run() 	xpcom/glue/nsThreadUtils.h
6 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
7 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
8 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
9 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
10 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc
11 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp
12 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c
13 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c
14 	msvcr100.dll 	_callthreadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:314
15 	msvcr100.dll 	_threadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:292

Comment 1

[(Away)]

Segfault during memcpy's read from |aSrc|.

Comment 2

[Matthew Gregan [:kinetik]]

The only change in AudioSink.cpp since it was created until Firefox 34 is bug 1037597, but that was also backported to 33, so it's probably not related.  There are a lot of changes to AudioStream in 34.

A way to reproduce, implicated URL, or regression window would be most welcome!

aSrc should be passed directly through from the AudioData instance in PlayFromAudioQueue.  Given the crash address, the AudioData isn't null (otherwise you'd expect to crash at 0 + small_offset), which implies that AudioData->mAudioData was released elsewhere.  I'm not sure how that's possible, since AudioData owns mAudioData and is responsible for releasing it (unless the decoder that created the AudioData violated the API and also freed the audio data?).  We should have the only reference to this AudioData in PlayFromWriteQueue, since AudioQueue.PopFront is thread-safe and the only other way to have a reference is via AudioQueue.PeekFront, which is never used to destroy an AudioData object (as far as I can see).

Given my vague suspicion of the decoder and the OS breakdown (Windows only, no XP), I'm wondering if this is related to changes in the WMF backend, but I have nothing else to support that.  ni? cpearce to see if anything comes to mind in that area for 34.

Comment 3

[(Away)]

Most of the reported URLs are ad iframes. I couldn't reproduce by visiting them at top-level.

The regression ranges tricky because the bug's appearance didn't really follow the trains. These may be wrong:

First appeared in nightly 20141001030205
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7c24470b6b3a&tochange=14665b1de5ee
Possible suspects: Bug 883731, Bug 1069660, Bug 1074004 

First appeared in aurora 20141007004003
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=c4a4b04c617c&tochange=92724a92ceaf
Possible suspects: Bug 1074048, Bug 1074420, Bug 1073792, Bug 1072780, Bug 1072775

Comment 4

[Matthew Gregan [:kinetik]]

Thanks!  Is there any common media type you're seeing being loaded?

Running with my (possibly incorrect) guess about this being related to WMF, bug 1073792 looks suspicious in that WMFAudioMFTMananger::UpdateOutputType may change mAudioRate and mAudioChannels, and WMFAudioMFTManager::Output computes values using those member variables before *and* after calling UpdateOutputType without handling the possibility that they've changed.  In this case, it seems to be possible for the size of the memory owned by the AudioData to be different to the length passed to it as numFrames, which is used in AudioStream::Write to compute bytesToCopy which flows into the memcpy in CircularByteBuffer::AppendElements.

Comment 5

[Matthew Gregan [:kinetik]]

I spun off bug 1091704 for comment 4.  It seems worth addressing.  If the crashes drop off, we'll know it was that.  Otherwise, :cpearce suggested investigating the prefing-on of the MP4Reader as the next potential culprit.

Comment 6

[(Away)]

I tried visiting some of the crash URLs while watching the network. I didn't see much media. I saw one mp4 file get requested, but it could just be coincidence. Hard to say from one data point.

This is also on the beta channel and spiked with the first 34 beta. (I didn't bother doing a regression range earlier since I figured it would just point to a merge day)

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0995ecffadcb

Comment 8

[Robert Kaiser]

Matthew, this crash is still high up in currebnt 34 betas, can you request uplifts to aurora and beta? Also, it looks like the patch and review haven't happened in here, what's the correct bug for that?

Comment 9

[Matthew Gregan [:kinetik]]

Sorry, bug number paste error.

I spun off bug 1096716 for comment 4.  That's now fixed in trunk, and we can certainly uplift it pretty safely.  I don't know that it fixes this topcrash (which is why I spun off the bug).  I was planning to leave this open until we had more information and/or confirmation of crashes dropping off after bug 1096716 landed.

Comment 10

[Robert Kaiser]

(In reply to Matthew Gregan [:kinetik] from comment #9)
> Sorry, bug number paste error.
> 
> I spun off bug 1096716 for comment 4.  That's now fixed in trunk, and we can
> certainly uplift it pretty safely.  I don't know that it fixes this topcrash
> (which is why I spun off the bug).  I was planning to leave this open until
> we had more information and/or confirmation of crashes dropping off after
> bug 1096716 landed.

Thanks. We'll need to land at least on Aurora, probably also on Beta, to find out if it actually helps as we have almost none of those crashes on Nightly in the first place.

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Kairo - The fix for bug 1096716 shipped in beta9. Do you have enough data yet to comment on the impact of that fix on this top crash?

Comment 12

[(Away)]

There was a dramatic decrease although about 10% remain.

Comparing against other signatures, I'd have expected a bit over 500 crashes, but there are only 50:
20141106201515 	1234
20141110195804 	1001
20141113192814 	50

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

I don't see this signature in 34.0b10 at all yet. I'll check back on the stats tomorrow and then maybe we can resolve this bug as fixed.

Comment 14

[Lawrence Mandel [:lmandel] (use needinfo)]

Liz - Can you please check on the stats and advise whether this bug can be resolved?

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Looks good, no crashes with this signature since Firefox 34.0b9.