Closed Bug 1310224 Opened 8 years ago Closed 8 years ago

OOM crash in output-only scenario on Windows/WASAPI

Categories

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

Product:
Core
Component:
Audio/Video: cubeb
Version:
50 Branch
Platform:
All
Windows
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
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)

Cherry-pick 46d12e9a from cubeb (for aurora)
3.88 KB, patch
padenot
: review+
gchang
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Cherry-pick 46d12e9a from cubeb (for beta)
4.05 KB, patch
padenot
: review+
jcristau
: approval-mozilla-beta+
Details | Diff | Splinter Review
Rank: 15
Priority: -- → P1
Depends on: 1310206
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+

          status-firefox53:
          --- → fixed
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: 8 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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: