1589197 - Crash in [@ mozilla::dom::FetchEventOp::MaybeFinished]

Status: RESOLVED WORKSFORME

(Core :: DOM: Service Workers, defect, P2)

Description

[Marcia Knous [:marcia]]

This bug is for crash report bp-e3b8b518-feaa-4017-a2d4-31be00191009.

Seen while looking at nightly crash stats: https://bit.ly/33CFevG. All reports have MOZ_DIAGNOSTIC_ASSERT(mRespondWithPromiseHolder.IsEmpty()) Linux and Windows crash which started in 20191009103354.

Bug 1456995 is in the regression range, but there are a few other service workers related landings in that changset:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=035f52aed4427b22facfa883067e298f10ef9e97&tochange=cad27f93a9869351457fc5d5290777420068e173

Top 10 frames of crashing thread:

0 xul.dll void mozilla::dom::FetchEventOp::MaybeFinished dom/serviceworkers/ServiceWorkerOp.cpp:1222
1 xul.dll nsresult mozilla::dom::FetchEventOp::DispatchFetchEvent dom/serviceworkers/ServiceWorkerOp.cpp:1638
2 xul.dll bool mozilla::dom::FetchEventOp::Exec dom/serviceworkers/ServiceWorkerOp.cpp:1237
3 xul.dll bool mozilla::dom::ServiceWorkerOp::ServiceWorkerOpRunnable::WorkerRun dom/serviceworkers/ServiceWorkerOp.cpp:276
4 xul.dll mozilla::dom::WorkerRunnable::Run dom/workers/WorkerRunnable.cpp:369
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1225
6 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
7 xul.dll mozilla::dom::WorkerPrivate::DoRunLoop dom/workers/WorkerPrivate.cpp:2872
8 xul.dll nsresult mozilla::dom::workerinternals::`anonymous namespace'::WorkerThreadPrimaryRunnable::Run dom/workers/RuntimeService.cpp:2330
9 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1225

Comment 1

[Marcia Knous [:marcia]]

Adding another signature that has the same moz crash reason.

Comment 2

[Jens Stutte [:jstutte]]

Bug 1231213 (https://phabricator.services.mozilla.com/D26172) introduced the failing MOZ_DIAGNOSTIC_ASSERT(mRespondWithPromiseHolder.IsEmpty());
What does this tell us?

Comment 3

[Perry Jiang [:perry] [no longer employee, use ni?]]

• MaybeFinished() is being called when synchronously checking how a Service Worker handles a FetchEvent (FetchEventOp::DispatchFetchEvent still being on the stack implies this).
• FetchEvent.respondWith definitely is called by the ServiceWorker. If it weren't, !fetchEvent->WaitToRespond() would cause the promise holder in the MOZ_DIAGNOSTIC_ASSERT to be "emptied".
• The MOZ_DIAGNOSTIC_ASSERT is only called if FetchEventOp::mResult contains some value, and this value must have been set by ExtendableEventKeepAliveHandler::MaybeDone when there are no pending promises, which seems to contradict that FetchEvent.respondWith is called...

Comment 4

[Julien Cristau [:jcristau]]

The last crash with this assertion was on the November 5 build. Did we change something?

Comment 5

[Perry Jiang [:perry] [no longer employee, use ni?]]

It's possible that some other Service Workers-related changes in the past 2 weeks fixed this as a side effect, but I can't make any direct connections between another bug and this one being fixed.

Comment 6

[Jim Mathies [:jimm]]

caused by a diagnostic assert

Comment 7

[Marcia Knous [:marcia]]

Trivial volume and based on Comment 6 it seems prudent to resolve this as WFM.