Closed Bug 1468241 Opened Last year Closed Last year

Crash in mozilla::Maybe<T>::value const


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

61 Branch



Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- fixed


(Reporter: marcia, Assigned: jya)


(Keywords: crash)

Crash Data


(2 files)

This bug was filed from the Socorro interface and is
report bp-82d26354-68c1-4638-9d2a-ef3fe0180601.

Seen while looking at nightly crash stats - crashes started using 20180531101443:

MOZ_RELEASE_ASSERT(mIsSome) is the crash reason. Maybe this is expected.

Top 10 frames of crashing thread:

0 mozilla::Maybe<mozilla::media::TimeUnit>::value const mfbt/Maybe.h:525
1 mozilla::RemoteVideoDecoder::IsUsefulData dom/media/platforms/android/RemoteDataDecoder.cpp:250
2 mozilla::RemoteDataDecoder::UpdateOutputStatus dom/media/platforms/android/RemoteDataDecoder.cpp:683
3 decltype  xpcom/threads/nsThreadUtils.h:1165
4 mozilla::detail::RunnableMethodImpl<mozilla::RemoteDataDecoder*, void  xpcom/threads/nsThreadUtils.h:1171
5 mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:243
6 nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:229
7 non-virtual thunk to nsThreadPool::Run xpcom/threads/nsThreadPool.cpp
8 nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1088
9 NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:519

Jean-Yves what do you think of this one? A recent regression?
Rank: 15
Flags: needinfo?(jyavenard)
Priority: -- → P2
It's a recent regression in that Maybe<T>::value() was changed to use MOZ_DIAGNOSTIC_ASSERT to make sure the Maybe contains a value.

The crash however is nonsensical because one line above:

we check that the value actually contains something.

So I don't see how that crash could ever happen with the code as it is.
You can't get to Maybe<mozilla::media::TimeUnit>::value if value isn't set...

unless the compiler for some reason optimised out the test...
Flags: needinfo?(jyavenard)
Oh I see the reason..

the code is racy:

Flush would reset mSeekTarget, but does so in the reader's taskqueue, while IsUsefulData is called on the decoder's task queue
Assignee: nobody → jyavenard
Useful when using promise chaining in combination with MozPromise::ResolveOrRejectValue parameter.

MozReview-Commit-ID: F8qMh7yFnHQ
Amend comments to clarify how some members are accessed.

MozReview-Commit-ID: FeEIRap3zvn

Depends on D1726
Comment on attachment 8986442 [details]
Bug 1468241 - P1. Add MozPromise::CreateAndResolveOrReject method

Gerald Squelart [:gerald] has approved the revision.
Attachment #8986442 - Flags: review+
Comment on attachment 8986444 [details]
Bug 1468241 - P2. Ensure RemoteVideoDecoder::mSeekTarget is only accessed on task queue.

Paul Adenot (:padenot) has approved the revision.
Attachment #8986444 - Flags: review+
Pushed by
P1. Add MozPromise::CreateAndResolveOrReject method r=gerald
Pushed by
P2. Ensure RemoteVideoDecoder::mSeekTarget is only accessed on task queue. r=padenot
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Looks like the racy code goes back to 57 or thereabouts? Either way, not convinced this is something we need to track for backporting anywhere, but let me know if you feel strongly otherwise :).
You need to log in before you can comment on or make changes to this bug.