Add-on manager / search service should block shutdown while installing add-ons to avoid "XPI database modified after shutdown began" errors
Categories
(Firefox :: Search, defect, P5)
Tracking
()
| Performance Impact | none |
People
(Reporter: Gijs, Unassigned)
References
Details
(Keywords: perf)
The initial PGO run logs lots of:
1558609217073 addons.xpi-utils WARN Error: XPI database modified after shutdown began(resource://gre/modules/addons/XPIDatabase.jsm:1307:17) JS Stack trace: saveChanges@XPIDatabase.jsm:1307:17
makeAddonVisible@XPIDatabase.jsm:2048:10
addToDatabase@XPIDatabase.jsm:1977:12
install@XPIInstall.jsm:3823:27
_activateAddon@XPIInstall.jsm:3842:7
async*installBuiltinAddon@XPIInstall.jsm:3778:16
async*XPIProvider[meth]@XPIProvider.jsm:2894:28
installBuiltinAddon@AddonManager.jsm:2037:33
installBuiltinAddon@AddonManager.jsm:3359:33
_loadEngines@SearchService.jsm:983:32
async*_init@SearchService.jsm:610:18
async*init@SearchService.jsm:1698:18
getDefault@SearchService.jsm:2232:16
delayedStartupInit@browser.js:3984:21
_delayedStartup@browser.js:1660:19
EventListener.handleEvent*onLoad@browser.js:1586:12
EventHandlerNonNull*@browser.xul:102:39
because as soon as the data: URI that we pass at https://searchfox.org/mozilla-central/rev/952521e6164ddffa3f34bc8cfa5a81afc5b859c4/build/pgo/profileserver.py#108-112 loads, we quit the browser, and that upsets the AM.
I'm not really sure what the right solution is here - I expect there might be both an AM bug (bits of this stuff should probably block shutdown) and an opportunity to update the quitter webext so it doesn't shut down until we get to post-idle-tasks in browser.js (after sessionstore-windows-restored).
Andrew, do you know if there's an issue with the AM here?
|
||
Comment 1•5 years ago
|
||
Is this the same as bug 1548515?
|
||
Comment 2•5 years ago
|
||
I don't think this is fatal, this is the search service trying to set up the default collection of search engines.
I'm not sure what the right fix is, I think that if the search service tries to start an install after the addon manager has shut down (which happens from profile-before-change) then the install will just fail. This sounds like the addon manager isn't handling the case where an install is in progress when it is shut down. Can we move this to an addon manager bug or should I file a new one?
|
||
Comment 3•5 years ago
|
||
Realistically, anything that depends on completing before shutdown should register its own shutdown blocker. I suppose we should probably add a shutdown barrier for AOM shutdown like we have for OS.File shutdown, but there are other usable barriers as it stands.
|
||
Updated•5 years ago
|
|
Reporter | |
Comment 4•5 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #2)
I don't think this is fatal, this is the search service trying to set up the default collection of search engines.
I'm not sure what the right fix is, I think that if the search service tries to start an install after the addon manager has shut down (which happens from profile-before-change) then the install will just fail. This sounds like the addon manager isn't handling the case where an install is in progress when it is shut down. Can we move this to an addon manager bug or should I file a new one?
I'm happy to use this bug to fix the AM / search service. Not sure who should be responsible for blocking shutdown, I haven't looked into it in detail.
(In reply to Michael Shal [:mshal] from comment #1)
Is this the same as bug 1548515?
Gah, didn't see that in the dupe search. Broadly yes. Let's use that bug to fix PGO/quitter.
|
||
Updated•5 years ago
|
|
||
Updated•5 years ago
|
|
||
Comment 5•5 years ago
|
||
After D32420 Bug 1552559 it should be easier to have the search service block shutdown until the extensions finish install...if that is what is desired.
|
|
Reporter | |
Comment 6•4 years ago
|
||
The stack in comment #0 is also around installing builtin add-ons, so maybe bug 1665150 will help here. Though tbf, we're already adding things to a list of startup promises - but maybe those don't block shutdown?
|
|
||
Comment 7•3 years ago
|
||
There's been no report around this in a long time, but it would need to be the search service that blocks shutdown when it is installing the builtin engines.
|
||
Updated•3 years ago
|
|
||
Updated•3 years ago
|
|
|
||
Updated•5 months ago
|
|
|
||
Comment 8•3 months ago
|
||
As of bug 1833829 the search service no longer depends on the add-on manager for application provided (aka built-in) search engines. Whilst we can still use third party engines, we don't try to install those during initialisation nor do we block on them.
Description
•