1403692 - ShutdownHang with CamerasParent::StopVideoCapture() blocking PBackground thread manifests as [Crash in AsyncShutdownTimeout | profile-before-change | ServiceWorkerRegistrar: Flushing data]

Status: RESOLVED DUPLICATE of bug 1407415

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-931d961e-72eb-46b6-90a4-809dd0170926.
=============================================================
###!!! ABORT: file /builds/worker/workspace/build/src/dom/workers/ServiceWorkerRegistrar.cpp, line 1027

these async shutdown timeout reports are starting to show up in firefox 57 across platforms. i haven't spotted the signature in 58 though...

Comment 1

[Kris Maglione [:kmag]]

I don't think this is caused by bug 1399646. I was already running into ServiceWorkerRegistrar shutdown issues before that bug, which is why I made those changes. Bug 1399646 just changes the failure mode and gives us better diagnostics.

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

My interpretation of comment 1 is that this is a signature change, instead of a new regression. However, it's still good to see if we know what happened according to the information newly added in bug 1399646. Any thoughts, Ben?

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Andrew, have you looked at this code recently?

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Ben Kelly [:bkelly] from comment #3)
> Andrew, have you looked at this code recently?

Yes, in bug 1402215 and previously I've ended up delving into AsyncShutdown a bit.  Taking, leaving needinfo for investigation response.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

It looks like the problem is a race involving content-process PBackground channel teardown and normal browser shutdown, with mozilla::camera::CamerasParent::StopVideoCapture() wedging the PBackground thread by blocking on a monitor.

Every report I've randomly sampled shows that the "IPDL Background" thread is stuck in `mozilla::camera::CamerasParent::StopVideoCapture()` on its mThreadMonitor.Wait() call.  This monitor in turn seems to be waiting on a VideoCaptureThread to shutdown.  This shutdown is triggered by PBackgroundParent::DestroySubtree() via MessageChannel::OnNotifyMaybeChannelError() thanks to someone/something calling MessageChannel::PostErrorNotifyTask().  Unfortunately because of the IsOnCxxStack() re-entrancy guard that uses a 10ms delay, we miss out on seeing some context on the source of the error.

We do, however, know that this isn't PBackground self-shutdown because PBackground only shuts itself down on "xpcom-shutdown-threads", which cannot have happened yet, because we're still in "profile-before-change".  Also, under e10s, the PCameras request would have come from a content process, not the main process.

So basically what happens is:
- ServiceWorkerRegistrar::ProfileStopped() gets invoked.
- We end up in the `!!child` path and we invoke BackgroundChild*::SendShutdownServiceWorkerRegistrar.
- BackgroundParentImpl::RecvShutdownServiceWorkerRegistrar never gets a chance to run because PBackground is janked hard by capture.

I'm not sure there's a clean mitigation that ServiceWorkerRegistrar can perform.  I'm going to file a new bug about CamerasParent::StopVideoCapture() blocking PBackground for content-child termination that blocks this one.  The method's comments hand-wave the monitor since the single code-path is used for both xpcom-shutdown and content child PBackground shutdown.  It also posits how "there will be no more possibility to get to the main thread", but I don't think that checks out given that you can always dispatch to the main thread because of deadlock concerns.  (The various thread methods are all like "oh, this is the main thread? yeah, that's cool.")

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

I can hit shutdownhang easily (near 100% reproduced rate) by opening many Indexeddb database, when I debug Bug 1411908 and Bug 1427238.

STR:
1. Open a tab; Go to weather.com
2. Load another web page which opens a lot of indexeddb databases. Like: https://shawnjohnjr.github.io/persist-demo/indexedbtest.html
3. Close browser.
4. Wait for shutdownhang.

Comment 7

[:gerard-majax]

I have been hitting that for a few days (roughly a week ?) on each shutdown to apply updates.

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late for a fix in 63. We can still take a patch in 65, and potentially in 64 beta.

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

This doesn't look as high volume in 64 release as it was in 63. 
It's still happening in 65 beta, though - I was able to reproduce it with the steps from comment 6: https://crash-stats.mozilla.com/report/index/b76b1322-4333-415d-8651-5fa8a0181220

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking this in bug 1407415 instead. Maybe we could dupe this to that bug.

Comment 11

[Neha Kochar [:neha]]

Andrew, are you actively working on this for 67?

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

No, just going to dupe this over to bug 1407415 as proposed in comment 10.