1543384 - AsyncShutdownTimeout timeout in "Startup: Run manifest"

Status: VERIFIED FIXED

(WebExtensions :: General, defect, P1)

Description

[Rob Wu [:robwu]]

Most if not all AsyncShutdownTimeout crash reports with async shutdown timeout at state "Startup: Run manifest" are unreliable, because the actual state is hidden.

That happens under the following circumstances:

1. source state is set to Startup: Emit startup,Run manifest
2. source this.runManifest may update state to Startup: Run manifest: <set of actual states>
3. source Management.emit("startup", this) completes and sets state to Startup: Run manifest

step 3 shouldn't have cleared the information from step 2. Reports may still include relevant information when the state from step 2 is recalculated at source.

This makes it considerably more difficult to evaluate the crash reports of bug 1464938.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1543384 - Fix race in extension state setter

Besides fixing the bug, a nice side effect of this patch is that it
becomes easier to add additional diagnostic information to AsyncShutdown
shutdown crashes inside an ExtensionAPI's onManifestEntry, via the new
extension.setStartupState and extension.deleteStartupState methods.

Comment 2

[Rob Wu [:robwu]]

STR for QA:

1. Run steps 1-3 from https://bugzilla.mozilla.org/show_bug.cgi?id=1543354#c2 (without step "4. Quit Firefox").
2. Run the code snippet from https://bugzilla.mozilla.org/show_bug.cgi?id=1543354#c3

Expected (with patch): One of the following:

• Startup: Run manifest, asyncEmitManifestEntry(\"background\")
• Startup: Run manifest: asyncEmitManifestEntry(\"background\")

Actual:

• Startup: Run manifest

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be1723043e97
Fix race in extension state setter r=kmag

Comment 4

[Cosmin Sabou [:CosminS]]

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

Comment 5

[Rob Wu [:robwu]]

Comment on attachment 9057366 [details]
Bug 1543384 - Fix race in extension state setter

Beta/Release Uplift Approval Request

• User impact if declined: AsyncShutdown crashes in bug 1464938 cannot be debugged optimally because about half of the crash reports have an incorrect signature. E.g. good signature vs bad signature in the past month.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: Optional, but if one wants to verify, see comment 2.
• 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 does not change functionality, only the way that crash signatures are generated.
• String changes made/needed: none

Comment 6

[Julien Cristau [:jcristau]]

Comment on attachment 9057366 [details]
Bug 1543384 - Fix race in extension state setter

extension fix, approved for 68.0b5

Comment 7

[Narcis Beleuzu [:NarcisB]]

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

Comment 8

[Alex Cornestean]

Hello,
We are currently validating the fix for bug https://bugzilla.mozilla.org/show_bug.cgi?id=1543384 and following the STR you mentioned yields a different result from what you mentioned. We get a "Startup: Complete" message (please check attachment).
We attempted to reproduce this on both Ubuntu and Windows with Nightly 69 and Beta 68, with the same results as above.
To detail the steps we took in reproducing the issue:

1. We changed the prefs in accordance to what you specified within https://bugzilla.mozilla.org/show_bug.cgi?id=1543354#c2
2. We installed netcat and configured it as mentioned (from the terminal in Ubuntu and from the command window which opens when running the .exe in Windows)
3. We loaded the provided extension in about:debugging
4. In the browser console we ran the code code snippet from https://bugzilla.mozilla.org/show_bug.cgi?id=1543354#c3

Is there anything we have done incorrectly, since we do not obtain the same results?

Comment 9

[Alex Cornestean]

Attachment: startup complete message.jpg

Comment 10

[Rob Wu [:robwu]]

(Clarified over Slack; in the execution of the STR, the server somehow wasn't listening; after changing to a different way of having a slow/never-responding server, the expected result is observed)

Comment 11

[Alex Cornestean]

Verified as Fixed using Windows 10 x64 and Ubuntu16.04 LTS on the Beta (68.0b5 - 20190527103257) and Nightly (69.0a1 – 20190528214841) versions of Firefox.

Following the provided STR, the results documented in the ‘Expected’ section of Comment 2 were correctly obtained, confirming the fix.

Performing additional testing using the latest Release version of Firefox (67.0 - Build ID 20190516215225), which is unaffected by the fix, confirmed the results documented in the ‘Actual’ section of Comment 2.