1530322 - Crash in [@ vcruntime140.dll | mozilla::AudioData::MoveableData]

Status: RESOLVED FIXED

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

Description

[Calixte Denizet (:calixte)]

This bug is for crash report bp-f3f7bb10-034a-4f66-b9f4-d42800190225.

Top 10 frames of crashing thread:

0 vcruntime140.dll vcruntime140.dll@0xc517 
1 xul.dll mozilla::AudioData::MoveableData dom/media/MediaData.cpp:154
2 xul.dll void mozilla::AudioSink::NotifyAudioNeeded dom/media/mediasink/AudioSink.cpp:419
3 xul.dll nsresult mozilla::AudioSink::Init dom/media/mediasink/AudioSink.cpp:84
4 xul.dll mozilla::AudioSinkWrapper::Start dom/media/mediasink/AudioSinkWrapper.cpp:170
5 xul.dll nsresult mozilla::VideoSink::Start dom/media/mediasink/VideoSink.cpp:190
6 xul.dll nsresult mozilla::MediaDecoderStateMachine::StartMediaSink dom/media/MediaDecoderStateMachine.cpp:3204
7 xul.dll void mozilla::MediaDecoderStateMachine::MaybeStartPlayback dom/media/MediaDecoderStateMachine.cpp:2851
8 xul.dll void mozilla::MediaDecoderStateMachine::DecodingState::Step dom/media/MediaDecoderStateMachine.cpp:2337
9 xul.dll nsresult mozilla::detail::RunnableMethodImpl< xpcom/threads/nsThreadUtils.h:1171

There is 1 crash in nightly 67 with buildid 20190225014816. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1524890.

[1] https://hg.mozilla.org/mozilla-central/rev?node=08fd6ddd88e4

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1530322 - P1. Make Interval assertion diagnostic_assert. r!gerald

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1530322 - P2. Remove redundant code, and simplify. r!gerald

The assertions were unneeded as the test above covered the case.

Depends on D21171

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1530322 - P3. Add temporary diagnostics. r?gerald

Depends on D21172

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68fd0947b767
P1. Make Interval assertion diagnostic_assert. r!gerald r=gerald
https://hg.mozilla.org/integration/autoland/rev/d5159dfc6d86
P2. Remove redundant code, and simplify. r!gerald r=gerald
https://hg.mozilla.org/integration/autoland/rev/65870fec6eea
P3. Add diagnostics. r=gerald

Comment 5

[Narcis Beleuzu [:NarcisB]]

Backed out for bustages on Assertions.h

Backout link: https://hg.mozilla.org/integration/autoland/rev/63242317442cedd9c5407b6fe5734c61036cb452
Push link: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=65870fec6eea2103719af73c7369d5242250ecdb
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=230484423&repo=autoland&lineNumber=24208

There were also AArch64 build failures.
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=230484421&repo=autoland&lineNumber=2062

Comment 6

[Jean-Yves Avenard [:jya]]

Interesting compilation error which has nothing to do with the code.
We replaced:
MOZ_ASSERT(aStart <= aEnd), with MOZ_DIAGNOSTIC_ASSERT(aStart <= aEnd)

However, above we had: mStart(std::forward<StartArg>(aStart)) so if aStart was a rvalue it could have been moved giving an undefined behaviour. Thanks to :gerald for spotting it.

Interestingly, it make gcc go cookoo.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f3ba8c155c6
P1. Make Interval assertion diagnostic_assert. r!gerald r=gerald
https://hg.mozilla.org/integration/autoland/rev/8ba89b2f60ff
P2. Remove redundant code, and simplify. r!gerald r=gerald
https://hg.mozilla.org/integration/autoland/rev/a5516893759b
P3. Add temporary diagnostics. r=gerald

Comment 8

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1530322 - Abort trim if an overflow occurred during earlier time calculation. r=padenot!

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/3f3ba8c155c6
https://hg.mozilla.org/mozilla-central/rev/8ba89b2f60ff
https://hg.mozilla.org/mozilla-central/rev/a5516893759b

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e715e094485f
Abort trim if an overflow occurred during earlier time calculation. r=padenot

Comment 11

[Jean-Yves Avenard [:jya]]

Adding the AudioData::SetTrimWindow signature, hoping to find the root cause of all those crashes with today's build

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/e715e094485f

Comment 13

[Pascal Chevrel:pascalc]

Tracking for 67 as this is supposed to fix bug 1530278 which is a top crasher.

Comment 14

[Calixte Denizet (:calixte)]

:jya, here is a crash report:
https://crash-stats.mozilla.com/report/index/26303278-e26a-4318-aae2-9d8720190227
where the the moz_crash_reason is MOZ_RELEASE_ASSERT(mStart <= mEnd) (Invalid Interval).

Comment 15

[Calixte Denizet (:calixte)]

Same moz_crash_reason, but different crash signature:
https://crash-stats.mozilla.com/report/index/b653bab3-0a4c-4a11-8136-6ac2d0190227

Comment 18

[Jean-Yves Avenard [:jya]]

(In reply to Calixte Denizet (:calixte) from comment #15)

Same moz_crash_reason, but different crash signature:
https://crash-stats.mozilla.com/report/index/b653bab3-0a4c-4a11-8136-6ac2d0190227

this one will be fixed by bug 1531201

Comment 20

[Jean-Yves Avenard [:jya]]

Marking it as fixed.

Leaving the DIAGNOSTIC_ASSERT as they are not showing up following bug 1531201 and bug 1531241