Closed Bug 1543384 Opened 8 months ago Closed 7 months ago

AsyncShutdownTimeout timeout in "Startup: Run manifest"

Categories

(WebExtensions :: General, defect, P1)

66 Branch
defect

Tracking

(firefox68 verified, firefox69 verified)

VERIFIED FIXED
mozilla69
Tracking Status
firefox68 --- verified
firefox69 --- verified

People

(Reporter: robwu, Assigned: robwu)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.

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.

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
Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/be1723043e97
Fix race in extension state setter r=kmag
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

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
Attachment #9057366 - Flags: approval-mozilla-beta?

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

extension fix, approved for 68.0b5

Attachment #9057366 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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?

Flags: needinfo?(rob)

(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)

Flags: needinfo?(rob)

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.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.