1563949 - Crash in [@ mozilla::media::TimeUnit::operator<T>]

Status: RESOLVED FIXED

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

Description

[Julian Seward [:jseward]]

This bug is for crash report bp-619de1a5-cea9-499c-9db7-59c380190706.

This crash occurs in 3 different installations for the Windows nightly of 20190704094530.

Top 10 frames of crashing thread:

0 xul.dll mozilla::media::TimeUnit::operator<= dom/media/TimeUnits.h
1 xul.dll const class mozilla::MediaRawData* mozilla::TrackBuffersManager::GetSample dom/media/mediasource/TrackBuffersManager.cpp:2532
2 xul.dll struct already_AddRefed<mozilla::MediaRawData> mozilla::TrackBuffersManager::GetSample dom/media/mediasource/TrackBuffersManager.cpp:2575
3 xul.dll class RefPtr<mozilla::MozPromise<RefPtr<mozilla::MediaTrackDemuxer::SamplesHolder>, mozilla::MediaResult, 1> > mozilla::MediaSourceTrackDemuxer::DoGetSamples dom/media/mediasource/MediaSourceDemuxer.cpp:426
4 xul.dll nsresult mozilla::detail::ProxyRunnable<mozilla::MozPromise<RefPtr<mozilla::MediaTrackDemuxer::SamplesHolder>, mozilla::MediaResult, 1>, RefPtr<mozilla::MozPromise<RefPtr<mozilla::MediaTrackDemuxer::SamplesHolder>, mozilla::MediaResult, 1> >  xpcom/threads/MozPromise.h:1313
5 xul.dll nsresult mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:199
6 xul.dll nsresult nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:244
7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1225
8 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
9 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:333

Comment 1

[Andrew McCreight [:mccr8]]

This is a checkedint assertion failure.
MOZ_RELEASE_ASSERT(mIsValid) (Invalid checked integer (division by zero or integer overflow))

Comment 2

[Nils Ohlmeier [:drno]]

Looks like a long standing low frequency assert crash.
Jean-Yves do you have a quick idea what could be going wrong here?

Comment 3

[Jean-Yves Avenard [:jya]]

This is no longer a MOZ_RELEASE_ASSERT, but a MOZ_DIAGNOSTIC_ASSERT

Here it's an overflow on aExpectedDts + aFuzz can easily fail that one before.

Assigning to :alwu as he's done similar work before.

Comment 4

[Alastor Wu [:alwu]]

Attachment: Bug 1563949 - part1 : add function 'GetEndTimeCode'.

We already have function GetEndTime(), so it's good to have a similar function GetEndTimeCode() so that we won't have to calculate end time code by ourselves.

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1563949 - part2 : add 'AppendSample' to assert that a sample should always be valid

It will be good to ensure all samples we use in the following media pipeline have valid time, duration, end time and end timecode.

So that we won't have to worry using any TimeUnit from MediaData with an invalid value.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Bug 1563949 - part4 : prevent direct usage of 'AppendElement()' to append sample.

We should always append sample by using AppendValidSample(), so making mSamples private can prevent direct a usage of the nsTarry's AppendElement() to append invalid sample.

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1563949 - part5 : return nullptr when time overflow.

If aExpectedDts or aExpectedPts is valid before adding Fuzz, but invalid after that, we should use the MAX or MIN boundary value as our bound to check if the sample is in the range.

Comment 8

[Alastor Wu [:alwu]]

Attachment: Bug 1563949 - part3 : handle invalid sample in demuxer.

Return demux error when we get a invalid sample.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5070c6dccc69
part1 : add function 'GetEndTimeCode'. r=jya
https://hg.mozilla.org/integration/autoland/rev/d259c4c81820
part2 : add 'AppendSample' to assert that a sample should always be valid r=jya
https://hg.mozilla.org/integration/autoland/rev/906d131e986c
part3 : handle invalid sample in demuxer. r=jya
https://hg.mozilla.org/integration/autoland/rev/ff9a24ac6c54
part4 : prevent direct usage of 'AppendElement()' to append sample. r=jya
https://hg.mozilla.org/integration/autoland/rev/9dbd561ce0e3
part5 : return nullptr when time overflow. r=jya

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/5070c6dccc69
https://hg.mozilla.org/mozilla-central/rev/d259c4c81820
https://hg.mozilla.org/mozilla-central/rev/906d131e986c
https://hg.mozilla.org/mozilla-central/rev/ff9a24ac6c54
https://hg.mozilla.org/mozilla-central/rev/9dbd561ce0e3