Closed Bug 1310224 Opened 3 years ago Closed 3 years ago

OOM crash in output-only scenario on Windows/WASAPI

Categories

(Core :: Audio/Video: cubeb, defect, P1)

50 Branch
All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- wontfix
firefox51 --- fixed
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: padenot, Assigned: padenot)

References

Details

(Keywords: regression)

Crash Data

Attachments

(2 files, 3 obsolete files)

Rank: 15
Priority: -- → P1
Crash Signature: [@ OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | auto_array<T>::reserve ]
Can you verify this is fixed?
Flags: needinfo?(padenot)
This still happens, around 40 crashes per day.

I'm thinking of bounding the array. Users might have glitches or clock drifts or something, but it's better than a crash for sure. It's very late for beta.
Flags: needinfo?(padenot)
Keywords: regression
Assignee: nobody → padenot
I just talked with padenot.  He's going to write the patch which will (likely) introduce a glitch as a first step to fixing this bug.  The reasoning is that a glitch is better than a crash.  But our goal is to find and resolve the root cause problem.  Not sure if we'll do all that on this one bug or break it into 2 bugs.
In fact, after looking at the code again, [0], that is upstream, will fix this.

What happens here is that we don't substract the number of frames that is still left unresampled, so it grows up after some time. I'll now prepare the patches for uplift, this is not very risky (famous last words).

[0]: https://github.com/kinetiknz/cubeb/commit/46d12e9ae6fa9c233bc32812b13185ee7df8d3fd
Attached patch Cherry-pick 46d12e9a from cubeb (obsolete) — Splinter Review
Attachment #8811368 - Flags: review?(kinetik)
MozReview-Commit-ID: Kkkyo6GSr6O
Attachment #8811373 - Flags: review?(kinetik)
Attachment #8811374 - Flags: review?(kinetik)
Attachment #8811368 - Attachment is obsolete: true
Attachment #8811368 - Flags: review?(kinetik)
Attachment #8811373 - Attachment description: Cherry-pick 46d12e9a from cubeb → Cherry-pick 46d12e9a from cubeb (beta patch)
Attachment #8811374 - Attachment description: Cherry-pick 46d12e9a from cubeb → Cherry-pick 46d12e9a from cubeb (aurora patch)
This has landed on inbound yesterday in bug 1314514.
Comment on attachment 8811373 [details] [diff] [review]
Cherry-pick 46d12e9a from cubeb (beta patch)

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

unresampled-frames.patch is missing, don't forget to include that on checkin.  r+ with that fixed.
Attachment #8811373 - Flags: review?(kinetik) → review+
Comment on attachment 8811374 [details] [diff] [review]
Cherry-pick 46d12e9a from cubeb (aurora patch)

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

Same problem.
Attachment #8811374 - Flags: review?(kinetik) → review+
Attachment #8811374 - Attachment is obsolete: true
MozReview-Commit-ID: D6wpucS71qh
Attachment #8811676 - Attachment is obsolete: true
Attachment #8811676 - Attachment description: Cherry-pick 46d12e9a from cubeb → Cherry-pick 46d12e9a from cubeb (for aurora)
Attachment #8811676 - Attachment is obsolete: false
Attachment #8811373 - Attachment is obsolete: true
Comment on attachment 8811677 [details] [diff] [review]
Cherry-pick 46d12e9a from cubeb (for beta)

Carrying r+ forward.
Attachment #8811677 - Attachment description: Cherry-pick 46d12e9a from cubeb → Cherry-pick 46d12e9a from cubeb (for beta)
Attachment #8811677 - Attachment filename: patch-aurora → patch-beta
Attachment #8811677 - Flags: review+
Comment on attachment 8811676 [details] [diff] [review]
Cherry-pick 46d12e9a from cubeb (for aurora)

Carrying r+ forward
Attachment #8811676 - Flags: review+
Paul -- can you ask for approval to get these patches into Aurora and Beta?
Flags: needinfo?(padenot)
Comment on attachment 8811676 [details] [diff] [review]
Cherry-pick 46d12e9a from cubeb (for aurora)

Approval Request Comment
[Feature/regressing bug #]: 1221572
[User impact if declined]: OOM crash
[Describe test coverage new/current, TreeHerder]: no crash since landed on nightly
[Risks and why]: low risk, cause and fix well understood.
[String/UUID change made/needed]: none
Flags: needinfo?(padenot)
Attachment #8811676 - Flags: approval-mozilla-aurora?
Comment on attachment 8811677 [details] [diff] [review]
Cherry-pick 46d12e9a from cubeb (for beta)

Approval Request Comment
[Feature/regressing bug #]: 1221572
[User impact if declined]: OOM crash
[Describe test coverage new/current, TreeHerder]: no crash since landed on nightly
[Risks and why]: low risk, cause and fix well understood.
[String/UUID change made/needed]: none
Attachment #8811677 - Flags: approval-mozilla-beta?
Comment on attachment 8811677 [details] [diff] [review]
Cherry-pick 46d12e9a from cubeb (for beta)

fix an oom crash in aurora52
Attachment #8811677 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1314514
Target Milestone: --- → mozilla53
Attachment #8811676 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Per comment #18, Beta51+. Should be in 51 beta 3.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
hi, the patch landed in 51.0b3, but there are still crashes there: https://crash-stats.mozilla.com/report/index/26b0a1ef-88f8-4f1d-b865-94c132161125
Flags: needinfo?(padenot)
Interesting, it OOMs when trying to allocate exactly (2 << 31) - 1 bytes of memory.
Our known windows duplex drift issue manifests itself as a crash sometimes here. I'm going to do something quick and easy to fix that up.
Flags: needinfo?(padenot)
You need to log in before you can comment on or make changes to this bug.