Closed
Bug 1033066
Opened 10 years ago
Closed 10 years ago
Large OOM coming from mozilla::AudioSegment::WriteTo
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: MatsPalmgren_bugz, Assigned: jesup)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
2.03 KB,
patch
|
karlt
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-e1050f6c-7076-48b0-af95-a95f52140624.
=============================================================
OOM Allocation Size: 4294930432
The number looks like a result from an integer overflow.
http://hg.mozilla.org/mozilla-central/annotate/f78e532e8a10/content/media/AudioSegment.cpp#l163
uint32_t outBufferLength = GetDuration() * outputChannels;
buf.SetLength(outBufferLength);
GetDuration() returns TrackTicks which is typedef int64_t.
Could it have been negative? or just very very large?
Perhaps this needs to be a fallible allocation?
Comment 1•10 years ago
|
||
FWIW, I sampled 13 version 33.0a1 crashes with this signature - all 13 have AudioSegment::WriteTo on the stack
(In reply to Mats Palmgren (:mats) from comment #0)
> report bp-e1050f6c-7076-48b0-af95-a95f52140624.
0 xul.dll NS_ABORT_OOM(unsigned int) xpcom/base/nsDebugImpl.cpp
1 xul.dll nsTArray_base<nsTArrayInfallibleAllocator,nsTArray_CopyWithMemutils>::EnsureCapacity(unsigned int,unsigned int) xpcom/glue/nsTArray-inl.h
2 xul.dll nsTArray_base<nsTArrayInfallibleAllocator,nsTArray_CopyWithMemutils>::InsertSlotsAt(unsigned int,unsigned int,unsigned int,unsigned int) xpcom/glue/nsTArray-inl.h
3 xul.dll nsTArray_Impl<float,nsTArrayInfallibleAllocator>::InsertElementsAt(unsigned int,unsigned int) obj-firefox/dist/include/nsTArray.h
4 xul.dll nsTArray_Impl<float,nsTArrayInfallibleAllocator>::SetLength(unsigned int) obj-firefox/dist/include/nsTArray.h
5 xul.dll mozilla::AudioSegment::WriteTo(unsigned __int64,mozilla::AudioStream *,mozilla::AudioMixer *) content/media/AudioSegment.cpp
6 xul.dll mozilla::MediaStreamGraphImpl::PlayAudio(mozilla::MediaStream *,__int64,__int64) content/media/MediaStreamGraph.cpp
7 xul.dll mozilla::MediaStreamGraphImpl::RunThread() content/media/MediaStreamGraph.cpp
8 xul.dll mozilla::`anonymous namespace'::MediaStreamGraphInitThreadRunnable::Run() content/media/MediaStreamGraph.cpp
Keywords: crashreportid
Comment 2•10 years ago
|
||
[Tracking Requested - why for this release]:
This is the #5 top crash in 32.0 (and 32.0.1) release. I checked a sample of reports from 32.0.1 and it's all AudioSegment stacks as well.
While it's lower in rank in beta right now, that can be due to different user populations on the channels as well as beta being dominated by graphics issues right now.
For more reports and stats, see https://crash-stats.mozilla.com/report/list?signature=OOM+%7C+large+%7C+NS_ABORT_OOM%28unsigned+int%29+%7C+nsTArray_base%3CnsTArrayInfallibleAllocator%2C+nsTArray_CopyWithMemutils%3E%3A%3AEnsureCapacity%28unsigned+int%2C+unsigned+int%29+%7C+nsTArray_base%3CnsTArrayInfallibleAllocator%2C+nsTArray_CopyWithMemutils%3E%3A%3AInsertSlotsAt%28unsigned%2E%2E%2E
OOMAllocationSize is always over 4294920000 Bytes, which very much supports what comment #0 says.
status-firefox32:
--- → affected
status-firefox33:
--- → affected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
tracking-firefox33:
--- → ?
Updated•10 years ago
|
Comment 3•10 years ago
|
||
Maire, given that Paul is out atm, can you find someone to take this?
Even if it's just 0.4% of 33 Beta crashes right now, it's a feature that probably isn't used too much yet and this doesn't sound like a nice case.
Flags: needinfo?(mreavy)
Updated•10 years ago
|
Assignee: nobody → rjesup
Flags: needinfo?(mreavy)
Updated•10 years ago
|
Summary: crash in OOM | large | NS_ABORT_OOM(unsigned int) | nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity(unsigned int, unsigned int) | nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::InsertSlotsAt(.. → Large OOM coming from mozilla::AudioSegment::WriteTo
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8494995 [details] [diff] [review]
Never let AudioSegments underflow mDuration and cause OOM allocation
Original assertion still needs to be dealt with, but it should never be allowed to OOM us
Attachment #8494995 -
Flags: review?(karlt)
Comment 7•10 years ago
|
||
Comment on attachment 8494995 [details] [diff] [review]
Never let AudioSegments underflow mDuration and cause OOM allocation
r+ on the AudioSegment::WriteTo() change, but please leave AudioChunk::SliceTo() as is or just upgrade the NS_ASSERTION to a MOZ_ASSERT.
SliceTo() has only two callers, both of which check that aStart < aEnd.
Attachment #8494995 -
Flags: review?(karlt) → review+
Comment 8•10 years ago
|
||
Randell, could you land this patch soon and request for uplift to aurora & beta? Thanks
Flags: needinfo?(rjesup)
Assignee | ||
Comment 10•10 years ago
|
||
Whiteboard: [leave-open][webrtc-uplift]
Target Milestone: --- → mozilla35
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8494995 [details] [diff] [review]
Never let AudioSegments underflow mDuration and cause OOM allocation
Approval Request Comment
[Feature/regressing bug #]: Not sure
[User impact if declined]: random crashes - seems to be tied to certain ?Facebook? games; may also happen more on lower-end stressed hardware or a buggy device. Never seen (or at least never noticed) it in automation.
[Describe test coverage new/current, TBPL]: none other than it will now assert on debug builds which may help catch it.
[Risks and why]: no risk - it's a pure wallpaper-over-bad-allocation. It will likely lead to an audio underrun (which happen for other reasons) and minor/momentary glitch/pop.
[String/UUID change made/needed]: none
Attachment #8494995 -
Flags: approval-mozilla-beta?
Attachment #8494995 -
Flags: approval-mozilla-aurora?
Comment 12•10 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #11)
> Approval Request Comment
Note that this is the #4 crash with 2% of all 32.0.3 crashes right now, so it would be really good to have this.
Comment 13•10 years ago
|
||
Comment 14•10 years ago
|
||
Comment on attachment 8494995 [details] [diff] [review]
Never let AudioSegments underflow mDuration and cause OOM allocation
Of course, I was just waiting for the patch to land in m-c.
Attachment #8494995 -
Flags: approval-mozilla-beta?
Attachment #8494995 -
Flags: approval-mozilla-beta+
Attachment #8494995 -
Flags: approval-mozilla-aurora?
Attachment #8494995 -
Flags: approval-mozilla-aurora+
Comment 15•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/f764e665b39c
https://hg.mozilla.org/releases/mozilla-beta/rev/82f4086ba2c7
Calling Fx33 and Fx34 fixed for tracking purposes. Assuming there's follow-up work to be done still, maybe we're better off moving it to a new bug?
Flags: needinfo?(rjesup)
Assignee | ||
Comment 16•10 years ago
|
||
Since this landed and uplifted, I see no reports that match it - the reports for newer builds are other bugs (appear to be OOMs opening files (URLClassifier DB, etc).
Flags: needinfo?(rjesup)
Assignee | ||
Updated•10 years ago
|
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [leave-open][webrtc-uplift] → [webrtc-uplift]
Assignee | ||
Updated•10 years ago
|
Whiteboard: [webrtc-uplift]
You need to log in
before you can comment on or make changes to this bug.
Description
•