Search service can install extensions after shutdown has started which makes things sad
Categories
(Firefox :: Search, defect, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox75 | --- | fixed |
People
(Reporter: Gijs, Assigned: standard8)
References
Details
Attachments
(1 file)
I spotted some errors in a log that indicated that we're trying to installBuiltinAddon after the addon manager shuts down, which ends in tears because it throws errors.
and
(that one is the one this is hitting, AFAICT)
should probably catch exceptions, and/or we should bail out if Services.startup.isShuttingDown is true or AddonManager.isReady is false.
|
Assignee | |
Comment 1•1 year ago
|
||
In both these cases we do actually have try/catches around these so the errors reported don't break init. We could add extra checks and avoid the console output, but I'm not completely sure that is worth it.
The concerning bit is that we'll still try to write the cache - we've seen it with reInit() (bug 1580737) that it could write a broken cache if we shutdown in the middle of reInit(). Broken here would be writing the cache but without the built-in search engines, which on next startup would mean that we think there's no built-in engines, and we wouldn't display them.
I missed the case here when fixing bug 1580737 that it could also apply to init(), so I think we should fix that. It may or may not help our cache problems that are tracked via bug 1589710.
|
||
Comment 2•1 year ago
|
||
(In reply to Mark Banner (:standard8) from comment #1)
It may or may not help our cache problems that are tracked via bug 1589710.
Sounds really plausible to me, so yay for finding it, Gijs!!
|
|
Assignee | |
Comment 3•1 year ago
|
||
I've just done a bit more testing. Hitting the "shutdown before init is finished" is quite easy. Hitting the add-on manager not started/shutting down case is quite difficult.
However, I still think it is worth the fix, and we have telemetry that is monitoring corruptions, so we should soon know if it helps or not.
|
|
Assignee | |
Comment 4•1 year ago
|
||
Abort it before writing the cache to avoid potentially writing a incomplete cache.
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a815e28f736a Handle shutdown occurring during SearchService initialisation. r=mikedeboer
|
|
Assignee | |
Updated•1 year ago
|
|
||
Comment 6•1 year ago
|
||
| bugherder | ||
|
||
Comment 7•1 year ago
|
||
Hi Mark, is there something that QA can verify here? Thanks!
|
|
Assignee | |
Comment 8•1 year ago
|
||
You could try a few tests along the lines of:
- Set up a Firefox profile where some of the default engines are removed / re-ordered / with different settings (e.g. keyword).
- Shut down Firefox.
- Start Firefox and close it as soon as you see the 'x' (assuming Windows here).
- Start Firefox again and check that the engines are still with the same settings.
- Repeat steps 3-4 varying the time after the window appears (you only need to go up to a max of a few seconds).
I'm not sure you'll be able to do a before / after comparison, this bug is highly timing dependent. However, just doing some tests along the lines of the above would be useful to reassure us it hasn't got worse.
|
||
Comment 10•1 year ago
|
||
Hello,
I tried to reproduce this issue with Fx 75.0a1 (BuildID: 20200305095541) and i could not get the timing down to reproduce this issue, on the plus side I could not reproduce with the current beta 75.0b10 either. I closed firefox 0.7 sec after the x button appeared and when I opened the browser again all the changes made were still there, I did this a couple of times and the settings were always saved.
I think it's safe to remove the qe+ flag for this bug based on comm 8.
Description
•