1824498 - Assertion failure: mResponseTarget->IsOnCurrentThread(), at /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:600

Status: VERIFIED FIXED

(Core :: DOM: Workers, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.zip

Found while fuzzing m-c 20230323-48de4270580b (--enable-address-sanitizer --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -a --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.zip

Assertion failure: mResponseTarget->IsOnCurrentThread(), at /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:600

#0 0x7fefe815aedb in mozilla::MozPromise<bool, nsresult, true>::ThenValueBase::DoResolveOrReject(mozilla::MozPromise<bool, nsresult, true>::ResolveOrRejectValue&) /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:600:7
#1 0x7fefe815a5d7 in mozilla::MozPromise<bool, nsresult, true>::ThenValueBase::ResolveOrRejectRunnable::Run() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:489:21
#2 0x7fefef600432 in mozilla::dom::(anonymous namespace)::ExternalRunnableWrapper::Cancel() /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:220:13
#3 0x7fefef5ef977 in mozilla::dom::WorkerRunnable::Run() /builds/worker/checkouts/gecko/dom/workers/WorkerRunnable.cpp:247:5
#4 0x7fefe721e3f4 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1233:16
#5 0x7fefe7217a57 in NS_ProcessPendingEvents(nsIThread*, unsigned int) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:442:19
#6 0x7fefef5dbeec in mozilla::dom::WorkerPrivate::ClearMainEventQueue(mozilla::dom::WorkerPrivate::WorkerRanOrNot) /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:3957:5
#7 0x7fefef5d7e9b in mozilla::dom::WorkerPrivate::NotifyInternal(mozilla::dom::WorkerStatus) /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:4824:7
#8 0x7fefef60201d in CloseInternal /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:3735:3
#9 0x7fefef60201d in mozilla::dom::(anonymous namespace)::CompileScriptRunnable::PostRun(JSContext*, mozilla::dom::WorkerPrivate*, bool) /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:451:23
#10 0x7fefef5ef780 in mozilla::dom::WorkerRunnable::Run() /builds/worker/checkouts/gecko/dom/workers/WorkerRunnable.cpp:401:3
#11 0x7fefe721e3f4 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1233:16
#12 0x7fefe72280a4 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:477:10
#13 0x7fefef5d671c in mozilla::dom::WorkerPrivate::DoRunLoop(JSContext*) /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:3280:7
#14 0x7fefef5ace12 in mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /builds/worker/checkouts/gecko/dom/workers/RuntimeService.cpp:2043:42
#15 0x7fefe721e3f4 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1233:16
#16 0x7fefe72280a4 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:477:10
#17 0x7fefe8a09ad4 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:300:20
#18 0x7fefe8885e67 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:381:10
#19 0x7fefe8885e67 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:374:3
#20 0x7fefe8885e67 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:356:3
#21 0x7fefe7215c95 in nsThread::ThreadFunc(void*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:391:10
#22 0x7ff00a104628 in _pt_root /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
#23 0x7ff009e94b42 in start_thread nptl/pthread_create.c:442:8
#24 0x7ff009f269ff  misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

Comment 1

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20230324165446-f476897a6e6a.
The bug appears to have been introduced in the following build range:

Start: 2508d061214d638aec40c04204c79e5e7f65c7f1 (20230118134745)
End: ad56233b442a0fd1514f1d257f68390ebc3527fc (20230118135404)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2508d061214d638aec40c04204c79e5e7f65c7f1&tochange=ad56233b442a0fd1514f1d257f68390ebc3527fc

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1247687

:yulia, since you are the author of the regressor, bug 1247687, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 3

[Yulia Startsev [:yulia]]

cc @jonco @allstarschh for looking at this while I am gone

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] OOO]

I was just debugging worklets with Grizzly Replay and can work on this now.

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] OOO]

So the problem is: the worker module script, worker.js, will import two non-existing scripts

// worker.js
import "foo";
import "bar";

And fetching "foo" triggers the ModuleError(), so worker.js cancels all the imports(), including the module "bar"
Then the loading of the worker.js is done (with failure), so WorkerScriptLoad is shutdown and closes the sync event loop.

The module "bar" is still being canceled, it rejects the promise (mReady) in ModuleLoadRequest, and this will create a Runnable.
When the Reject Runnable starts to run, it will be canceled, and call ExternalRunnableWrapper::Cancel,
and WorkerPrivate::EventTarget::IsOnCurrentThreadInfallible returns false because it has been shutdown, and causes the assert in MozPromise.

Comment 6

[Jon Coppeard (:jonco)]

(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] from comment #5)
Can we delay shutdown until cancellation has finished?

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] OOO]

(In reply to Jon Coppeard (:jonco) from comment #6)

Can we delay shutdown until cancellation has finished?

I have WIP patches to process the remaining runnables before the shutdown, however it causes shutdown hang so I am still investigating on that.

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] OOO]

Attachment: Bug 1824498 - Call shutdown when there isn't any ongoing fetching modules.

In the test file, worker_bug1824498.mjs, it imports two module scripts,
"foo" and "bar", both are invalid specifiers.

The ModuleLoadRequest of "foo" will be canceled, which in turn will:

1. Cancel all imports of the parent ModuleLoadRequest (worker_bug1824498.mjs)
2. Cancel the ModuleLoadRequest of "bar".

After the step 1, WorkerModuleLoader::OnModuleLoadComplete will be
called, and will shutdown the script loader.

The shutdown causes two problems:

1. When step 2 is executed, it will reject the mReady promise in
   ModuleLoadRequest, however when the MozPromise is dispatched, its event
   target has been shutdown so an assertion failure is triggered.

2. Also when the ScriptLoaderRunnable of "bar" is received, it also
   triggers the assertion failure of the valid SyncLoopEventTarget.

To fix the problem, we delay the shutdown until there's no ongoing
fetching modules.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

This bug prevents fuzzing from making progress; however, it has low severity. It is important for fuzz blocker bugs to be addressed in a timely manner (see here why?).
:allstars.chh, could you increase the severity?

For more information, please visit auto_nag documentation.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bfff24ed14cb
Call shutdown when there isn't any ongoing fetching modules. r=jonco

Comment 11

[Cosmin Sabou [:CosminS]]

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

Comment 12

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20230421030443-df954b717c6c.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 13

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] OOO]

Comment on attachment 9328550 [details]
Bug 1824498 - Call shutdown when there isn't any ongoing fetching modules.

Beta/Release Uplift Approval Request

• User impact if declined: Assertion failure in debug build
  and SEGV fault in opt build when a worker module tries to import more than one invalid module.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch counts the number of total ongoing module requests and uses this information to decide shutdown WorkerScriptLoader.
• String changes made/needed: no
• Is Android affected?: Yes

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9328550 [details]
Bug 1824498 - Call shutdown when there isn't any ongoing fetching modules.

Approved for 113.0b7.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a1d3a3567d36