1404441 - Assertion failure: mIsValid (Invalid checked integer (division by zero or integer overflow)) [@ mozilla::ReaderProxy::RequestVideoData]

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.mp4

Assertion failure: mIsValid (Invalid checked integer (division by zero or integer overflow)), at /src/obj-firefox/dist/include/mozilla/CheckedInt.h:591

#0 mozilla::CheckedInt<long>::value() const /src/obj-firefox/dist/include/mozilla/CheckedInt.h:592:12
#1 mozilla::media::TimeUnit::IsNegative() const /src/dom/media/TimeUnits.h:114:43
#2 mozilla::MediaData::AdjustForStartTime(long) /src/dom/media/MediaData.h:334:19
#3 mozilla::ReaderProxy::RequestVideoData(mozilla::media::TimeUnit const&)::$_2::operator()(RefPtr<mozilla::VideoData>) const /src/dom/media/ReaderProxy.cpp:96:22
#4 _ZN7mozilla10MozPromiseI6RefPtrINS_9VideoDataEENS_11MediaResultELb1EE12InvokeMethodIZNS_11ReaderProxy16RequestVideoDataERKNS_5media8TimeUnitEE3$_2MSC_KFS1_IS5_ES3_ES3_EENS_8EnableIfIXsr13TakesArgumentIT0_EE5valueENS_6detail11MethodTraitISH_E10ReturnTypeEE4TypeEPT_SH_OT1_ /src/obj-firefox/dist/include/mozilla/MozPromise.h:520:12
#5 mozilla::EnableIf<true, void>::Type mozilla::MozPromise<RefPtr<mozilla::VideoData>, mozilla::MediaResult, true>::InvokeCallbackMethod<true, mozilla::ReaderProxy::RequestVideoData(mozilla::media::TimeUnit const&)::$_2, RefPtr<mozilla::MozPromise<RefPtr<mozilla::VideoData>, mozilla::MediaResult, true> > (mozilla::ReaderProxy::RequestVideoData(mozilla::media::TimeUnit const&)::$_2::*)(RefPtr<mozilla::VideoData>) const, RefPtr<mozilla::VideoData>, RefPtr<mozilla::MozPromise<RefPtr<mozilla::VideoData>, mozilla::MediaResult, true>::Private> >(mozilla::ReaderProxy::RequestVideoData(mozilla::media::TimeUnit const&)::$_2*, RefPtr<mozilla::MozPromise<RefPtr<mozilla::VideoData>, mozilla::MediaResult, true> > (mozilla::ReaderProxy::RequestVideoData(mozilla::media::TimeUnit const&)::$_2::*)(RefPtr<mozilla::VideoData>) const, RefPtr<mozilla::VideoData>&&, RefPtr<mozilla::MozPromise<RefPtr<mozilla::VideoData>, mozilla::MediaResult, true>::Private>&&) /src/obj-firefox/dist/include/mozilla/MozPromise.h:544:14
#6 mozilla::MozPromise<RefPtr<mozilla::VideoData>, mozilla::MediaResult, true>::ThenValue<mozilla::ReaderProxy::RequestVideoData(mozilla::media::TimeUnit const&)::$_2, mozilla::ReaderProxy::RequestVideoData(mozilla::media::TimeUnit const&)::$_3>::DoResolveOrRejectInternal(mozilla::MozPromise<RefPtr<mozilla::VideoData>, mozilla::MediaResult, true>::ResolveOrRejectValue&) /src/obj-firefox/dist/include/mozilla/MozPromise.h:765:9
#7 mozilla::MozPromise<RefPtr<mozilla::VideoData>, mozilla::MediaResult, true>::ThenValueBase::ResolveOrRejectRunnable::Run() /src/obj-firefox/dist/include/mozilla/MozPromise.h:402:21
#8 mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() /src/obj-firefox/dist/include/mozilla/TaskDispatcher.h:209:37
#9 mozilla::TaskQueue::Runner::Run() /src/xpcom/threads/TaskQueue.cpp:246:12
#10 nsThreadPool::Run() /src/xpcom/threads/nsThreadPool.cpp:226:14
#11 non-virtual thunk to nsThreadPool::Run() /src/xpcom/threads/nsThreadPool.cpp:154:15
#12 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14
#13 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:524:10
#14 mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:368:5
#15 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#16 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#17 nsThread::ThreadFunc(void*) /src/xpcom/threads/nsThread.cpp:427:11
#18 _pt_root /src/nsprpub/pr/src/pthreads/ptthread.c:216:5
#19 start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
#20 clone /build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

I got a shutdown hang in osx.

Repro steps:
1. /mach run https://bugzilla.mozilla.org/attachment.cgi\?id\=8913788
2. wait for the first frame to load
3. command+Q to quit Firefox

Result: shutdown hang in MediaShutdownManager.

LLDB shows that one of the threads got stuck in VTDecompressionSessionWaitForAsynchronousFrames() called by

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

LLDB shows that one of the threads got stuck in VTDecompressionSessionWaitForAsynchronousFrames() called by AppleVTDecoder::ProcessFlush().

Comment 3

[Jean-Yves Avenard [:jya]]

Then, there's not much we can do about it.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Only reproduces on Linux for me.

INFO: Last good revision: 7ab7f74c36a233444da15a80448e7045b086ad5b
INFO: First bad revision: e8babe547652de8cfb5aa274487151f848fab093
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=7ab7f74c36a233444da15a80448e7045b086ad5b&tochange=e8babe547652de8cfb5aa274487151f848fab093

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

http://searchfox.org/mozilla-central/rev/1a4a26905f923458679a59a4be1e455ebc53c333/dom/media/ReaderProxy.cpp#96

aVideo::mTime is 9223372036854770905us and startTime is -4906us. aVideo->AdjustForStartTime(startTime) will do 9223372036854770905 - (-4906) and result in int64_t overflow.

Bug 1356530 changed the type of mTime from int64_t to TimeUnit to make the overflow observable.

Hi Jya,
aVideo::mTime is valid (insanely large though) before adjustment and becomes invalid after adjusted by the start time. What is the best practice to handle such bad data? Should we just detect overflow in MediaData::AdjustForStartTime()?

Comment 6

[Jean-Yves Avenard [:jya]]

Sorry for the late answer, I didn't know what to answer with, and still can't today.

Even if we handled the AdjustForStartTime differently, we would still hit the issue in the Apple VT decoder one way or another.

We could maybe just handle those type of overflow as a decoding error and handle it as such... (but would still hit the problem of Apple VT)

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Hm... Chrome can't play the file at all. I think we should just throw an error if MediaData::AdjustForStartTime() results in an invalid mTime.

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1404441 - return NS_ERROR_DOM_MEDIA_OVERFLOW_ERR if MediaData::AdjustForStartTime() results in an invalid mTime.

Review commit: https://reviewboard.mozilla.org/r/208346/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208346/

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8937650 [details]
Bug 1404441 - return NS_ERROR_DOM_MEDIA_OVERFLOW_ERR if MediaData::AdjustForStartTime() results in an invalid mTime.

https://reviewboard.mozilla.org/r/208346/#review214086

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

Thanks!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5bc55868583b
return NS_ERROR_DOM_MEDIA_OVERFLOW_ERR if MediaData::AdjustForStartTime() results in an invalid mTime. r=jya

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/5bc55868583b

Comment 13

[Julien Cristau [:jcristau]]

wontfix for 58 since we've shipped this in at least 56 and 57 and it's late in the cycle.  Feel free to reset to affected and request uplift if you disagree.