Firefox crash in mozilla::SourceBufferDecoder::WasTrimmed()

VERIFIED FIXED in Firefox 36

Status

()

--
critical
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: marcia, Assigned: mattwoodrow)

Tracking

(Blocks: 1 bug, {crash})

36 Branch
mozilla38
x86
Windows 7
crash
Points:
---

Firefox Tracking Flags

(firefox35 unaffected, firefox36+ verified, firefox37+ verified, firefox38 verified)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-10052775-fb3f-4e72-bc68-ede142150130.
=============================================================

Seen while looking at the Aurora explosive report, but this appears to be happening on Nightly as well as the most recent beta 5 which has yet to be pushed out (ID:20150129200438)

Crashes across all branches: http://bit.ly/1yYn0BQ

There are crashes dating back to build ID 2015012503, and crashes as recent as 2015013003 on nightly

Comments:
* Firefox crashs when i open videos on youtube using the html5 player 
* Youtube crashes 100% of the time. Normal HTML5 videos works correctly, for example http://www.w3.org/2010/05/video/mediaevents.html 
* just started firefox and visited youtube, when clicked a video to play the browser crashed. 
* when youtube is open on a tab, and open a new tab = crash 


rame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::SourceBufferDecoder::WasTrimmed() 	dom/media/mediasource/SourceBufferDecoder.h
1 	xul.dll 	mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, __int64) 	dom/media/mediasource/TrackBuffer.cpp
2 	xul.dll 	mozilla::dom::SourceBuffer::AppendData(mozilla::LargeDataBuffer*, double) 	dom/media/mediasource/SourceBuffer.cpp
3 	xul.dll 	mozilla::dom::AppendDataRunnable::Run() 	dom/media/mediasource/SourceBuffer.cpp
4 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
5 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
6 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
7 	xul.dll 	nsThreadManager::GetMainThread(nsIThread**) 	xpcom/threads/nsThreadManager.cpp
8 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
9 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp
10 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp
11 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
12 	xul.dll 	nsAString_internal::Replace(unsigned int, unsigned int, wchar_t const*, unsigned int, mozilla::fallible_t const&) 	xpcom/string/nsTSubstring.cpp
13 	xul.dll 	nsLocalFile::AppendInternal(nsString const&, bool) 	xpcom/io/nsLocalFileWin.cpp
14 	xul.dll 	nsLocalFile::Append(nsAString_internal const&) 	xpcom/io/nsLocalFileWin.cpp
15 	kernel32.dll 	GetProcessPriorityBoost 	
16 	kernel32.dll 	GetCurrentProcess 	
17 	xul.dll 	std::basic_string<char, std::char_traits<char>, std::allocator<char> >::_Tidy(bool, unsigned int) 	c:/tools/vs2013/vc/include/xstring:2282
18 	xul.dll 	`anonymous namespace'::HistogramGet(char const*, char const*, unsigned int, unsigned int, unsigned int, unsigned int, bool, base::Histogram**) 	toolkit/components/telemetry/Telemetry.cpp
19 	xul.dll 	`anonymous namespace'::HistogramGet(char const*, char const*, unsigned int, unsigned int, unsigned int, unsigned int, bool, base::Histogram**) 	toolkit/components/telemetry/Telemetry.cpp
20 	xul.dll 	mozilla::TimeStamp::ProcessCreation(bool&) 	xpcom/ds/TimeStamp.cpp
21 		@0x2e9a7f
This one will be fixed with bug 1125776.

But I can make a quicker fix if need be.
(Assignee)

Comment 2

4 years ago
Posted patch Crash fixSplinter Review
Are you planning on uplifting the other bug to beta? If not (or not by monday), then we should take this.
Attachment #8557574 - Flags: review?(jyavenard)
Comment on attachment 8557574 [details] [diff] [review]
Crash fix

Review of attachment 8557574 [details] [diff] [review]:
-----------------------------------------------------------------

I had this in my queue..

The fundamental issue is that it shows that you can get there with mCurrentDecoder being nullptr ; it should never have been the case as we must have received an init segment by now and a decoder would have been created.

As far as pushing my changes, I'm waiting for reviews...
Attachment #8557574 - Flags: review?(jyavenard) → review+
(Assignee)

Comment 4

4 years ago
This can happen if initialization of the decoder failed, which is the case for bug 1128013.
(In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> This can happen if initialization of the decoder failed, which is the case
> for bug 1128013.

Then this is not the proper fix... it will prevent the crash, but things will crap out after that.

     if (!decoders.NewDecoder(aTimestampOffset)) {
       return false;
     }

should be:
     if (!decoders.NewDecoder(aTimestampOffset)) {
       mParser = ContainerParser::CreateForMIMEType(mType);
       return false;
     }

so we never attempt to present a buffered range we can't play... and further call to appendBuffer will abort early (currently , as soon as we've seen an init segment, we proceed forward)
(Assignee)

Comment 6

4 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #5)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> > This can happen if initialization of the decoder failed, which is the case
> > for bug 1128013.
> 
> Then this is not the proper fix... it will prevent the crash, but things
> will crap out after that.
> 
>      if (!decoders.NewDecoder(aTimestampOffset)) {
>        return false;
>      }
> 
> should be:
>      if (!decoders.NewDecoder(aTimestampOffset)) {
>        mParser = ContainerParser::CreateForMIMEType(mType);
>        return false;
>      }
> 
> so we never attempt to present a buffered range we can't play... and further
> call to appendBuffer will abort early (currently , as soon as we've seen an
> init segment, we proceed forward)

NewDecoder doesn't fail, ReadMetadata does (which calls DiscardDecoder).
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1128295
Crash Signature: [@ mozilla::SourceBufferDecoder::WasTrimmed()] → [@ mozilla::SourceBufferDecoder::WasTrimmed()] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, __int64)]
This report was pretty explosive in Beta 5, so anything we can do to try to improve things in the next Beta would be great.

Updated

4 years ago
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED

Updated

4 years ago
Assignee: jyavenard → matt.woodrow
https://hg.mozilla.org/mozilla-central/rev/6037ec1faa35
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38

Comment 11

4 years ago
[Tracking Requested - why for this release]:
Nominating for tracking 36 as this is ~3.2% of b5 crashes.
status-firefox36: --- → affected
tracking-firefox36: --- → ?
Matt, can we have an uplift request? Thanks
tracking-firefox36: ? → +
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 13

4 years ago
Comment on attachment 8557574 [details] [diff] [review]
Crash fix

Approval Request Comment
[Feature/regressing bug #]: Bug 1096089
[User impact if declined]: Crashes on some videos
[Describe test coverage new/current, TreeHerder]: Few days on m-c
[Risks and why]: Very low risk, just a null check
[String/UUID change made/needed]: None
Flags: needinfo?(matt.woodrow)
Attachment #8557574 - Flags: approval-mozilla-beta?
Attachment #8557574 - Flags: approval-mozilla-aurora?
status-firefox35: --- → unaffected
status-firefox37: --- → affected
status-firefox38: --- → fixed
tracking-firefox37: --- → +
Comment on attachment 8557574 [details] [diff] [review]
Crash fix

Trivial fix for crashes. Beta+ Aurora+
Attachment #8557574 - Flags: approval-mozilla-beta?
Attachment #8557574 - Flags: approval-mozilla-beta+
Attachment #8557574 - Flags: approval-mozilla-aurora?
Attachment #8557574 - Flags: approval-mozilla-aurora+
Crash Signature: [@ mozilla::SourceBufferDecoder::WasTrimmed()] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, __int64)] → [@ mozilla::SourceBufferDecoder::WasTrimmed()] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, __int64)] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuffer*, long long)] [@ mozilla::TrackBuffer::AppendData(mozilla::LargeDataBuf…
Socorro shows no more crashes after the fix landed.
Status: RESOLVED → VERIFIED
status-firefox36: fixed → verified
status-firefox37: fixed → verified
status-firefox38: fixed → verified
You need to log in before you can comment on or make changes to this bug.