Closed Bug 1091704 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Audio/Video, defect)

34 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 + fixed
firefox35 + fixed
firefox36 + fixed

People

(Reporter: lizzard, Assigned: kinetik)

References

Details

(Keywords: crash, topcrash-win)

Crash Data

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
Segfault during memcpy's read from |aSrc|.
Flags: needinfo?(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.
Flags: needinfo?(kinetik) → needinfo?(cpearce)
Keywords: steps-wanted
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
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.
Depends on: 1096716
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.
Flags: needinfo?(cpearce)
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)
https://hg.mozilla.org/mozilla-central/rev/0995ecffadcb
Assignee: nobody → kinetik
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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?
Flags: needinfo?(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.
Assignee: kinetik → nobody
Status: RESOLVED → REOPENED
Flags: needinfo?(kinetik)
Resolution: FIXED → ---
(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.
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?
Flags: needinfo?(kairo)
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
Flags: needinfo?(kairo)
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.
Liz - Can you please check on the stats and advise whether this bug can be resolved?
Looks good, no crashes with this signature since Firefox 34.0b9.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Assignee: nobody → kinetik
You need to log in before you can comment on or make changes to this bug.