AsyncShutdown timeout in asyncEmitManifestEntry("chrome_settings_overrides")

VERIFIED FIXED in Firefox 65

Status

defect
P1
normal
VERIFIED FIXED
9 months ago
4 months ago

People

(Reporter: kmag, Assigned: mkaply)

Tracking

(Blocks 1 bug, {crash, regression})

unspecified
mozilla66
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox63 wontfix, firefox64 wontfix, firefox65 verified, firefox66 verified, firefox67 verified)

Details

(crash signature)

Attachments

(1 attachment)

Reporter

Description

9 months ago
This happens in particular with the DuckDuckGo search add-on:

{
    "conditions": [
        {
            "filename": "resource://gre/modules/addons/XPIProvider.jsm",
            "lineNumber": 2211,
            "name": "Extension shutdown: jid1-ZAdIEUB7XOzOJw@jetpack",
            "stack": ["resource://gre/modules/addons/XPIProvider.jsm:startup/<:2211"],
            "state": {"state": "Startup: Run manifest: asyncEmitManifestEntry(\"chrome_settings_overrides\")"}
        }
    ],
    "phase": "profile-change-teardown"
}
Flags: needinfo?(mozilla)
Assignee

Comment 1

9 months ago
Can you give me some more details as to what this means?
Flags: needinfo?(mozilla)
Reporter

Comment 2

9 months ago
(In reply to Mike Kaply [:mkaply] from comment #1)
> Can you give me some more details as to what this means?

The promise returned by the onManifest handler in the chrome_settings_override API never resolves for some users.
Assignee

Comment 4

9 months ago
In the second case, the add-on was Ecosia.

What's odd is these are two very different scenarios. In the DuckDuckGo case, no engine is added (because it already exists).

In the second case, an engine is added

Where did your information come for the opening the original problem?

Is this something you were able to recreate?
Priority: -- → P1
From offline conversation with mkaply:

Which suggests that one of the search-related promises isn't resolving in time (ie "await searchInitialized" or "await addSearchEngine()".  Is there some possible case where the search service "init-complete" event never gets fired?
Assignee: nobody → mozilla
Assignee

Comment 6

7 months ago
Have a recreation scenario on Windows:

Create a new profile and make sure it doesn't show the "multiple tabs open" message on shutdown.

1. Use the web page method to install an add-on which includes a search engine (eg. https://ext.ask.com/index.jhtml)
2. When the second prompt is showing, which is for the default search, close Firefox using the X button in the window title bar

If anything interferes with the closing of Firefox, it won't happen (dialog popup).

What you'll see is that multiple processes are left open in the task manager and after a minute or so, Firefox crashes

Updated

7 months ago
Crash Signature: [@ AsyncShutdownTimeout | profile-change-teardown | Extension shutdown: _j5Members_@ext.ask.com] [@ AsyncShutdownTimeout | profile-change-teardown | Extension shutdown: search@mail.ru] [@ AsyncShutdownTimeout | profile-change-teardown | Extension shutdo…
Keywords: crash, regression

Comment 8

5 months ago
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/6c39cc812c87
Switch WebExt default search prompt away from promise. r=aswan

Comment 9

5 months ago
bugherder
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Is this something we might consider taking in a 65 RC build or is it safer at this point to just let it ride with 66?

Flags: needinfo?(mozilla)
Reporter

Comment 11

5 months ago

This is super trivial. I'd suggest taking it.

Assignee

Comment 12

5 months ago

I'm personally torn. On the one hand, it hasn't been seen a lot, on the other hand it's a crasher.

As Kris said, it's a pretty trivial patch.

Flags: needinfo?(mozilla)
Assignee

Comment 13

5 months ago

Comment on attachment 9035795 [details]
Bug 1493770 - Switch WebExt default search prompt away from promise.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: Occasional crash when extension with search engine is installed.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: See https://bugzilla.mozilla.org/show_bug.cgi?id=1493770#c6

Works on Mac too.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Refactors very self contained code.

String changes made/needed:

Attachment #9035795 - Flags: approval-mozilla-release?
Flags: qe-verify+

This issue appears to only be reproducible on Windows platform. Is this correct?

In any case, the issue was reproduced on Nightly v66.0a1 from 20190115 and Windows 10 and the fix is now verified on Nightly v66
.0a1 from 20190123 and Windows 10, using the str form comment 6. I will mark this bug as verified.

@Mike: Could this issue also be reproduced on other OSes than Windows? Could/Should I verify the fix on other OSes?

Thank you!

Status: RESOLVED → VERIFIED
Flags: needinfo?(mozilla)
Assignee

Comment 15

5 months ago

It is recreatable on Mac (you have to use the keystroke to close the app).

But Windows was most prevalent, so seeing that you are able to verify the fix worked, I think we're good.

Flags: needinfo?(mozilla)

Comment on attachment 9035795 [details]
Bug 1493770 - Switch WebExt default search prompt away from promise.

[Triage Comment]
Fixes an occasional crash when an extension with a search engine is installed. Verified by QA on nightly. Approved for 65.0 RC2.

Attachment #9035795 - Flags: approval-mozilla-release? → approval-mozilla-release+

Comment 18

5 months ago

i don't really see a decline of those crashes in 65.0rc2 - should we reopen the bug or follow up in a new one?
https://crash-stats.mozilla.com/search/?async_shutdown_timeout=~chrome_settings_overrides&version=65.0rc2

Reporter

Comment 20

5 months ago

The MOZ_CRASH reason is now sanitized, and I don't have access to unsanitized probes, so I can't even begin to guess whether the crashes are still from this issue, or from another.

(In reply to Mike Kaply [:mkaply] from comment #19)

Are those this bug?

https://bugzilla.mozilla.org/show_bug.cgi?id=1464938

Bug 1464938 is an umbrella bug for all extension shutdown hangs, including this one.

Reporter

Comment 21

5 months ago

OK, apparently there's an AsyncShutdownTimeout metadata field with the necessary information now. And this is definitely still a bug in chrome_settings_overrides, though perhaps in a different place now.

As an addition to comment 14, I have reproduced the issue on Mac OS 10.13.6 with Nightly v66.0a1 from 2019-01-15 (using the COMMAND + Q keystroke to quit the browser, and then verified the fix in Nightly v67.0a1 from 2019-01-30.

Furthermore, while continuing testing for uplift, the changeset from comment 17 is to the release channel, not to beta. I have reproduced the issue in Firefox Release v64.0 and then validated the fix on Firefox Release v65.0 on Windows 10 and Mac OS 10.13.6.
I have also verified the fix on the latest Nightly for certainty.

Thank you for your contribution!

Flags: qe-verify+

Updated

5 months ago
See Also: → 1525729
Depends on: 1534796
You need to log in before you can comment on or make changes to this bug.