1635408 - Abort pending worker tasks at shutdown if they do not change state

Status: RESOLVED FIXED

(Firefox :: Remote Settings Client, defect)

Description

[:Gijs (he/him)]

The worker supports 4 different operations:

• canonicalStringify
• importJSONDump
• checkContentHash
• checkFileHash

All of them except importJSONDump do not write any data, so we should not wait for them at shutdown; we should just stop doing them and shut down the worker, and propagate the error.

At time of writing, there are no more reports of shutdown hangs relating to importJSONDump after bug 1630766 landed on nightly, other than these 2 crashreports:

https://crash-stats.mozilla.org/report/index/27523fde-2ec8-41fd-bd00-d5aad0200505#tab-metadata
https://crash-stats.mozilla.org/report/index/c18caeb0-919c-4bf0-95da-599160200504#tab-metadata

In both of them, the only tasks the worker is busy with are file/content hash checks.

It's possible that more reports that do reference importJSONDump will surface, but at least for now it seems prudent to deal with the low-hanging fruit of aborting these tasks first.

Comment 1

[:Gijs (he/him)]

Attachment: Bug 1635408 - abort non-critical RemoteSettingsWorker tasks at shutdown, r?leplatrem

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/819c1fb3e554
abort non-critical RemoteSettingsWorker tasks at shutdown, r=leplatrem

Comment 3

[Razvan Maries]

Backed out for xpcshell perma failures.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=NIOfMPPLQb2fdG0DYd3n2w-0&resultStatus=testfailed%2Cbusted%2Cexception&revision=819c1fb3e5544ec5c7beaa1dfe32528267d75854

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=300877470&repo=autoland&lineNumber=2376

Backout: https://hg.mozilla.org/integration/autoland/rev/259b47282ffcb160de444b50c2ebaf417aeedf13

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5db356e1bb94
abort non-critical RemoteSettingsWorker tasks at shutdown, r=leplatrem

Comment 5

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/5db356e1bb94

Comment 6

[:Gijs (he/him)]

Comment on attachment 9145799 [details]
Bug 1635408 - abort non-critical RemoteSettingsWorker tasks at shutdown, r?leplatrem

Beta/Release Uplift Approval Request

• User impact if declined: Shutdown hangs leading to shutdown crashes
• 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: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a relatively small JS-only change that improves the shutdown handling of the remote settings worker existing shutdown blocker. We're seeing hundreds of these hangs/crashes on release, and I'd like us to nip those in the bud (and, if there remain significant numbers of 77 crashes, to have time to address them in 78 given it's an ESR cycle). I'd like this to land together with the patch in bug 1636256 (that patch requires this one, but not the other way around). Since the changes in this bug and bug 1636256, the crashes on nightly have reduced to 0, but it's hard to be sure how strong the effect will be on beta/release given the difference in number of users.
• String changes made/needed: none

Comment 7

[Pascal Chevrel:pascalc]

Comment on attachment 9145799 [details]
Bug 1635408 - abort non-critical RemoteSettingsWorker tasks at shutdown, r?leplatrem

Approve for 77 beta 7, thanks.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/5e1017a6eef8