1841702 - Hit MOZ_CRASH(MozPromise::ThenValue created from 'BeginFinishing' destroyed without being either disconnected, resolved, or rejected (dispatchRv: not dispatched)) at /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:534

Status: RESOLVED FIXED

(Core :: DOM: File, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20230628-0df511b69760 (--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.html

Hit MOZ_CRASH(MozPromise::ThenValue created from 'BeginFinishing' destroyed without being either disconnected, resolved, or rejected (dispatchRv: not dispatched)) at /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:534

#0 0x7fd6fbcb3652 in MOZ_Crash /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:281:3
#1 0x7fd6fbcb3652 in mozilla::MozPromise<mozilla::void_t, mozilla::ipc::ResponseRejectReason, true>::ThenValueBase::AssertIsDead() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:529:11
#2 0x7fd6fbcb29ae in mozilla::MozPromise<mozilla::void_t, mozilla::ipc::ResponseRejectReason, true>::AssertIsDead() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:1141:13
#3 0x7fd6fa43b632 in ~ResolveOrRejectRunnable /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:484:23
#4 0x7fd6fa43b632 in mozilla::MozPromise<bool, bool, false>::ThenValueBase::ResolveOrRejectRunnable::~ResolveOrRejectRunnable() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:482:34
#5 0x7fd6fa478014 in Release /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:66:1
#6 0x7fd6fa478014 in Release /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:88:1
#7 0x7fd6fa478014 in Release /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:91:1
#8 0x7fd6fa478014 in mozilla::PrioritizableCancelableRunnable::Release() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:151:1
#9 0x7fd704a9c91d in ~nsCOMPtr /builds/worker/workspace/obj-build/dist/include/nsCOMPtr.h:340:7
#10 0x7fd704a9c91d in ~ExternalRunnableWrapper /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:196:38
#11 0x7fd704a9c91d in mozilla::dom::(anonymous namespace)::ExternalRunnableWrapper::~ExternalRunnableWrapper() /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:196:38
#12 0x7fd704a9c584 in Release /builds/worker/checkouts/gecko/dom/workers/WorkerRunnable.cpp:218:1
#13 0x7fd704a9c584 in mozilla::dom::(anonymous namespace)::ExternalRunnableWrapper::Release() /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:193:3
#14 0x7fd6fa45f8d0 in assign_assuming_AddRef /builds/worker/workspace/obj-build/dist/include/nsCOMPtr.h:318:7
#15 0x7fd6fa45f8d0 in operator= /builds/worker/workspace/obj-build/dist/include/nsCOMPtr.h:593:5
#16 0x7fd6fa45f8d0 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1210:13
#17 0x7fd6fa45895d in NS_ProcessPendingEvents(nsIThread*, unsigned int) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:445:19
#18 0x7fd704a34ed9 in mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /builds/worker/checkouts/gecko/dom/workers/RuntimeService.cpp:2198:9
#19 0x7fd6fa45f819 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1193:16
#20 0x7fd6fa46ce24 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#21 0x7fd6fc0843f1 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:300:20
#22 0x7fd6fbeae97a in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:370:10
#23 0x7fd6fbeae97a in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#24 0x7fd6fbeae97a in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#25 0x7fd6fa45680a in nsThread::ThreadFunc(void*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:391:10
#26 0x7fd7212c8b3f in _pt_root /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
#27 0x7fd721094b42 in start_thread nptl/pthread_create.c:442:8
#28 0x7fd7211269ff  misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

Comment 1

[Tyson Smith [:tsmith]]

Bug 1840184 was marked as a fuzzblocker until m-c 20230628-3b7b3970a884.
As of m-c 20230628-0df511b69760 this bug is now the fuzzblocker.

Comment 2

[Bugmon [:jkratzer for issues]]

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

Start: 53b4b785ae2a7f70257069e77b138fe36b53698a (20230612211509)
End: eb926a42fef72714535ed99bfa3586454449f898 (20230612192238)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=53b4b785ae2a7f70257069e77b138fe36b53698a&tochange=eb926a42fef72714535ed99bfa3586454449f898

Comment 3

[Dianna Smith [:diannaS]]

:jstutte based on the above regression range can you check if this is due to bug 1777921?

Comment 4

[Jens Stutte [:jstutte]]

BeginFinishing creates a series of chained promises.

Bug 1777921 just increased the number of GC/CC runs during worker shutdown. That can trigger releases that otherwise would have happened later (or never?). The testcase shows a nasty sequence of:

    let a = await e.data[0].getFileHandle("c21deba4-fb73-4407-94f8-2e3782bf3f23", {"create": true})
    self.close()
    await a.createWritable({})

in the worker, IIRC we will process also (some) JS after self.close() has been called so we might also see a promise creation from createWriteable happening while/after we started to shutdown the worker.

Jari, Jan, you were involved in creating that promise chain and Eden may be able to help with knowledge about canceling runnables during worker shutdown. There might be something fundamental between promises and worker shutdown here we would better want to understand.

Comment 5

[Jan Varga [:janv]]

It seems, we should add something like this:

  if (aManager->IsShutdown()) {
    aError.Throw(NS_ERROR_ILLEGAL_DURING_SHUTDOWN);
    return nullptr;
  }

to FileSystemFileHandle::CreateWritable for start

Comment 6

[Jan Varga [:janv]]

I'm working on this.

Comment 7

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

Based on comment #2, this bug contains a bisection range found by bugmon. However, the Regressed by field is still not filled.

:janv, if possible, could you fill the Regressed by field and investigate this regression?

For more information, please visit BugBot documentation.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Jan Varga [:janv] from comment #5)

aError.Throw(NS_ERROR_ILLEGAL_DURING_SHUTDOWN);

Nit, we should not expose Gecko internal error messages through web APIs.

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?).
:janv, could you consider increasing the severity?

For more information, please visit BugBot documentation.

Comment 10

[Jan Varga [:janv]]

Attachment: Bug 1841702 - Add proper handling for failed build worker refs; r=#dom-storage

Comment 11

[Jan Varga [:janv]]

Attachment: Bug 1841702 - Add a new test; r=#dom-storage

Depends on D187428

Comment 12

[Jan Varga [:janv]]

https://treeherder.mozilla.org/jobs?repo=try&revision=17a4b5c2cb6160a4960ef86c81534d0c33ecff4b

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/98bc951d34c3
Add proper handling for failed build worker refs; r=dom-storage-reviewers,jari

Comment 14

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/98bc951d34c3

Comment 15

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

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 16

[Bugmon [:jkratzer for issues]]

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

Comment 17

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

The patch landed in nightly and beta is affected.
:janv, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8db4cef31be
Add a new test; r=dom-storage-reviewers,jari

Comment 19

[Jan Varga [:janv]]

Hm, maybe we need to check StrongWorkerRef::Create here as well:
https://searchfox.org/mozilla-central/rev/253125d1947acbc3033b7b2e3a9a0d1bf4358a2d/dom/fs/child/FileSystemRequestHandler.cpp#301

Comment 20

[Sandor Molnar[:smolnar]]

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

Comment 21

[Jan Varga [:janv]]

Attachment: Bug 1841702 - Add proper handling for failed worker refs; r=#dom-storage

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4f429e902f8
Add proper handling for failed worker refs; r=dom-storage-reviewers,jari

Comment 23

[Narcis Beleuzu [:NarcisB]]

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