1757618 - Windows Crash in [@ OOM | large | mozalloc_abort | moz_xmalloc | mozilla::SPSCRingBufferBase<T>::SPSCRingBufferBase]

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

~20 crashes per beta since 98.0b7, 9 crashes with Firefox 99.0a1, 70% with 32-bit builds, all on Windows

Filing as security bug because the Android bugs 1756473 and bug 1756473 will be set as these as a precaution.

Crash report: https://crash-stats.mozilla.org/report/index/9a18dcb9-419e-48a5-86b4-fe37f0220301

MOZ_CRASH Reason: out of memory: 0x00000000000BB804 bytes requested

Top 10 frames of crashing thread:

0 mozglue.dll mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:26
1 mozglue.dll moz_xmalloc memory/mozalloc/mozalloc.cpp:58
2 xul.dll mozilla::SPSCRingBufferBase<float>::SPSCRingBufferBase mfbt/SPSCQueue.h:114
3 xul.dll mozilla::AudioSink::AudioSink dom/media/mediasink/AudioSink.cpp:60
4 xul.dll mozilla::AudioSinkWrapper::CreatorImpl<`lambda at /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:2868:27'>::Create dom/media/mediasink/AudioSinkWrapper.h:42
5 xul.dll mozilla::AudioSinkWrapper::Start dom/media/mediasink/AudioSinkWrapper.cpp:174
6 xul.dll mozilla::VideoSink::Start dom/media/mediasink/VideoSink.cpp:219
7 xul.dll mozilla::MediaDecoderStateMachine::StartMediaSink dom/media/MediaDecoderStateMachine.cpp:3460
8 xul.dll mozilla::MediaDecoderStateMachine::MaybeStartPlayback dom/media/MediaDecoderStateMachine.cpp:3052
9 xul.dll mozilla::MediaDecoderStateMachine::DecodingState::Step dom/media/MediaDecoderStateMachine.cpp:2447

Comment 1

[Paul Adenot (:padenot)]

Just a regular OOM, I'll make this try a smaller size. Not a sec bug.

Comment 2

[Paul Adenot (:padenot)]

Attachment: Bug 1757618 - Use smaller ring buffer on anything but m1 macs. r?#media-playback-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47dc3e2e467e
Use smaller ring buffer on anything but m1 macs. r=media-playback-reviewers,alwu

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/47dc3e2e467e

Comment 5

[Donal Meehan [:dmeehan]]

:padenot this looks like a good candidate for a beta uplift request?

Comment 6

[Paul Adenot (:padenot)]

Yes.

Comment 7

[Paul Adenot (:padenot)]

Comment on attachment 9266320 [details]
Bug 1757618 - Use smaller ring buffer on anything but m1 macs. r?#media-playback-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: OOM crash on Windows, especially on 32-bits
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is just a pref flip that lowers an allocation size everywhere but macOS with an aarch64 CPU. This has been on nightly for a few days without issues.
• String changes made/needed:

Comment 8

[Donal Meehan [:dmeehan]]

Comment on attachment 9266320 [details]
Bug 1757618 - Use smaller ring buffer on anything but m1 macs. r?#media-playback-reviewers

Approved for 99.0b4. Thanks.

Comment 9

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/990096832ebe

Comment 10

[Ryan VanderMeulen [:RyanVM]]

This looks like something we probably want to uplift to 98 for a dot release. Please nominate for approval ASAP if so.

Comment 11

[Paul Adenot (:padenot)]

Comment on attachment 9266320 [details]
Bug 1757618 - Use smaller ring buffer on anything but m1 macs. r?#media-playback-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: OOM crash on Windows, especially on 32-bits
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is just a pref change that lowers an allocation size everywhere but macOS with an aarch64 CPU. This has been on nightly for a few days without issues, and has been uplifted to beta.
• String changes made/needed:

Comment 12

[Pascal Chevrel:pascalc]

Comment on attachment 9266320 [details]
Bug 1757618 - Use smaller ring buffer on anything but m1 macs. r?#media-playback-reviewers

Approved for 98.0.2, thanks.

Comment 13

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-release/rev/7e44790a3693