1891129 - Assertion failure: !mAudio.mSeekRequest.Exists(), at /dom/media/MediaFormatReader.cpp:3107

Status: RESOLVED WONTFIX

(Core :: Audio/Video, defect)

Description

[Jason Kratzer [:jkratzer]]

Testcase found while fuzzing mozilla-central rev 008989a6a743 (built with: --enable-debug --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build 008989a6a743 --debug --fuzzing  -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

Assertion failure: !mAudio.mSeekRequest.Exists(), at /dom/media/MediaFormatReader.cpp:3107

    ==18711==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x761946463246 bp 0x76193c353650 sp 0x76193c353590 T18757)
    ==18711==The signal is caused by a WRITE memory access.
    ==18711==Hint: address points to the zero page.
        #0 0x761946463246 in mozilla::MediaFormatReader::DoAudioSeek() /dom/media/MediaFormatReader.cpp:3107:3
        #1 0x76194646447c in mozilla::MediaFormatReader::OnVideoSeekCompleted(mozilla::media::TimeUnit) /dom/media/MediaFormatReader.cpp:3050:5
        #2 0x7619465874c2 in InvokeMethod<mozilla::MediaFormatReader, void (mozilla::MediaFormatReader::*)(mozilla::media::TimeUnit), mozilla::media::TimeUnit> /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:651:12
        #3 0x7619465874c2 in InvokeCallbackMethod<false, mozilla::MediaFormatReader, void (mozilla::MediaFormatReader::*)(mozilla::media::TimeUnit), mozilla::media::TimeUnit, RefPtr<mozilla::MozPromise<mozilla::media::TimeUnit, mozilla::MediaResult, true>::Private> > /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:682:5
        #4 0x7619465874c2 in mozilla::MozPromise<mozilla::media::TimeUnit, mozilla::MediaResult, true>::ThenValue<mozilla::MediaFormatReader*, void (mozilla::MediaFormatReader::*)(mozilla::media::TimeUnit), void (mozilla::MediaFormatReader::*)(mozilla::MediaResult const&)>::DoResolveOrRejectInternal(mozilla::MozPromise<mozilla::media::TimeUnit, mozilla::MediaResult, true>::ResolveOrRejectValue&) /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:732:9
        #5 0x761946328105 in mozilla::MozPromise<mozilla::media::TimeUnit, mozilla::MediaResult, true>::ThenValueBase::ResolveOrRejectRunnable::Run() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:487:21
        #6 0x76194265db68 in mozilla::TaskQueue::Runner::Run() /xpcom/threads/TaskQueue.cpp:257:20
        #7 0x761942688f19 in nsThreadPool::Run() /xpcom/threads/nsThreadPool.cpp:341:14
        #8 0x76194267f411 in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1193:16
        #9 0x7619426863ed in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:480:10
        #10 0x76194338ea9e in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:300:20
        #11 0x7619432a3861 in RunHandler /ipc/chromium/src/base/message_loop.cc:363:3
        #12 0x7619432a3861 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:345:3
        #13 0x76194267a6e3 in nsThread::ThreadFunc(void*) /xpcom/threads/nsThread.cpp:370:10
        #14 0x76195779866f in _pt_root /nsprpub/pr/src/pthreads/ptthread.c:201:5
        #15 0x76195803aac2 in start_thread nptl/pthread_create.c:442:8
        #16 0x7619580cc84f  misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
    
    UndefinedBehaviorSanitizer can not provide additional info.
    SUMMARY: UndefinedBehaviorSanitizer: SEGV /dom/media/MediaFormatReader.cpp:3107:3 in mozilla::MediaFormatReader::DoAudioSeek()
    ==18711==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Testcase

Comment 2

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit BugBot documentation.

Comment 3

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20240506094855-65e120a9c1dc.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: 25209607dc0b253635ead03eff51fdc10a62e659 (20230508094425)
End: 008989a6a7435d8ee5ead586b2e444eed16b51c2 (20240411042626)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False)

Successfully recorded a pernosco session. A link to the pernosco session will be added here shortly.

Comment 4

[Paul Adenot (:padenot)]

Would it be possible to have the pernosco link here? Thanks!

Comment 5

[Bugmon [:jkratzer for issues]]

A pernosco session for this bug can be found here.

Comment 6

[Paul Adenot (:padenot)]

alwu, somethings weird when looping and seekToNextFrame, here's an in-progress pernosco session: https://pernos.co/debug/5fMr4CbvsJ1YpZJZ7L2n8Q/index.html#f{m[CAGC,BsyP_,t[Ags,U6U_,f{e[CAGC,BsyI_,s{afwifq5AA,bAdo,uC3VIFw,oC5WD1g___/ with some calls merged as an attempt to understand what's going on. It seems to me that at some point, we perform an audio-only seek request while the previous one hasn't completed.

With Web Codecs coming, we should remove seekToNextFrame(), or make it test-only. We can do this in https://bugzilla.mozilla.org/show_bug.cgi?id=1336404. If we remove it, then we don't need to fix this bug and we can simplify the code greatly I assume.

What do you think?

Comment 7

[Alastor Wu [:alwu]]

Sure I will take a look, keep NI.

Comment 8

[Alastor Wu [:alwu]]

(In reply to Paul Adenot (:padenot) from comment #6)

With Web Codecs coming, we should remove seekToNextFrame(), or make it test-only. We can do this in https://bugzilla.mozilla.org/show_bug.cgi?id=1336404. If we remove it, then we don't need to fix this bug and we can simplify the code greatly I assume.

What do you think?

Yes, I agree that we should remove seekToNextFrame() which is not a standard method.

Comment 9

[Alastor Wu [:alwu]]

when doing a normal seeking, we always reset decode to ensure all previous data request would be discarded. But for next frame seeking, we didn't do anything like that.

As the data requests from the previous state might still be running, if we start requesting another new seeking before them finish, then it would hit the assertion. Instead of adding similar steps into next frame seeking, it would probably be better to evaluate whether seekToNextFrame() is still needed. If not, then we can remove that function and fix this problem. WDYT?

Comment 10

[Paul Adenot (:padenot)]

I've attached a patch to bug 1336404. When it lands, we can close this.

Comment 11

[Bugmon [:jkratzer for issues]]

No valid actions for resolution (WONTFIX).
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.