Closed Bug 1277295 Opened 8 years ago Closed 8 years ago

Extension installation failing with NS_ERROR_XPC_GS_RETURNED_FAILURE for Services.storage

Categories

(Toolkit :: Add-ons Manager, defect)

46 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla50
Tracking Status
firefox47 --- affected
firefox48 --- affected
firefox49 --- affected
firefox50 --- verified

People

(Reporter: cs_gon, Assigned: aswan)

References

Details

(Whiteboard: triaged)

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160425115534

Steps to reproduce:

I tried to install the Stylish extension from addons.mozilla.org in FF 46 (on Ubuntu 14.04). Downloading and the installation worked fine, and then FF told me it needs to restart.


Actual results:

Before the restart the extension was shown in the add-ons list, but after the restart the extension wasn't there any more.

The following error was shown on restart:

1464793560318	addons.xpi	ERROR	Unable to read add-on manifest from /u/m525025/.mozilla/firefox/7c14k6yq.default/extensions/staged/{46551EC9-40F0-4e47-8E18-8E5CF550CFB8}.xpi: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: resource://gre/modules/XPCOMUtils.jsm :: XPCU_serviceLambda :: line 230"  data: no] Stack trace: XPCU_serviceLambda()@resource://gre/modules/XPCOMUtils.jsm:230 < XPCU_defineLazyGetter/<.get()@resource://gre/modules/XPCOMUtils.jsm:198 < defineSyncGUID()@resource://gre/modules/addons/XPIProvider.jsm:1254 < loadManifestFromZipReader<()@resource://gre/modules/addons/XPIProvider.jsm:1433 < TaskImpl_run()@resource://gre/modules/Task.jsm:315 < Handler.prototype.process()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:933 < this.PromiseWalker.walkerLoop()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:812 < this.PromiseWalker.scheduleWalkerLoop/<()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:746 < syncLoadManifestFromFile()@resource://gre/modules/addons/XPIProvider.jsm:1489 < this.XPIProvider.processPendingFileChanges()@resource://gre/modules/addons/XPIProvider.jsm:3279 < this.XPIProvider.checkForChanges()@resource://gre/modules/addons/XPIProvider.jsm:3568 < this.XPIProvider.startup()@resource://gre/modules/addons/XPIProvider.jsm:2657 < callProvider()@resource://gre/modules/AddonManager.jsm:227 < _startProvider()@resource://gre/modules/AddonManager.jsm:833 < AddonManagerInternal.startup()@resource://gre/modules/AddonManager.jsm:1016 < this.AddonManagerPrivate.startup()@resource://gre/modules/AddonManager.jsm:2782 < amManager.prototype.observe()@resource://gre/components/addonManager.js:58





Expected results:

The extension should have been installed.

Btw. this also happened in FF 45. But after downgrading to FF 40 I was able to install Stylish, and then after upgrading FF to version 46, Stylish was still installed and working correctly. 

I think I have had the same problem with other extensions, which I tried to install, but cannot say if the same problem (output in terminal) had occured. I also treid startgin with a new user profile, but this didn't change anything.
Are you able to install and use this add-on with a fresh profile?
https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles
Flags: needinfo?(cs_gon)
As I said in the last sentence (btw., sorry for the typos), I tried it with a new profile (deleted the whole ~/.mozilla folder), but the problem is still there.
Flags: needinfo?(cs_gon)
Component: Untriaged → Add-ons Manager
Product: Firefox → Toolkit
Wfm in Fx 46.0.1, Mac 10.11.5.
Wfm in Fx 46.0.1, Ubuntu 14.04 as well.
I had some hopes after the last comments, but unfortunately I still have the same problem with FF 46.0.1.
This sounds a lot like https://bugzilla.mozilla.org/show_bug.cgi?id=717904

In any case, this is the code that imports that storage service that is failing: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#1227-1229

I suspect this code dates back to when the add-on database was in sqlite instead of json, and I'm inclined to just remove it.  (If this line really is pivotal to ensuring things get loaded in the right order, it probably should be somewhere else, not buried in XPIProvider.jsm!)
Dave, does this seem reasonable?  Any guidance on how much coverage to go for in a try run?
Flags: needinfo?(dtownsend)
Summary: Cannot install extensions (e.g. Stylish) in newer FF (45/46) → Extension installation failing with NS_ERROR_XPC_GS_RETURNED_FAILURE for Services.storage
Hitting precisely same issue in BlueGriffon 2.0 on Windows. I try/catch'd the
storage line and it works as expected.
This problem seems to also affect bundled FF extensions. After trying out the (upstream) FF 47, I get this warning/error on every launch:

1465377475731	addons.xpi-utils	WARN	updateMetadata: Add-on firefox@getpocket.com is invalid: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: resource://gre/modules/XPCOMUtils.jsm :: XPCU_serviceLambda :: line 230"  data: no] Stack trace: XPCU_serviceLambda()@resource://gre/modules/XPCOMUtils.jsm:230 < XPCU_defineLazyGetter/<.get()@resource://gre/modules/XPCOMUtils.jsm:198 < defineSyncGUID()@resource://gre/modules/addons/XPIProvider.jsm:1205 < loadManifestFromZipReader<()@resource://gre/modules/addons/XPIProvider.jsm:1384 < TaskImpl_run()@resource://gre/modules/Task.jsm:319 < Handler.prototype.process()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937 < this.PromiseWalker.walkerLoop()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816 < this.PromiseWalker.scheduleWalkerLoop/<()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:750 < syncLoadManifestFromFile()@resource://gre/modules/addons/XPIProvider.jsm:1440 < updateMetadata()@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:1776 < processFileChanges()@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:1973 < this.XPIProvider.checkForChanges()@resource://gre/modules/addons/XPIProvider.jsm:3620 < this.XPIProvider.startup()@resource://gre/modules/addons/XPIProvider.jsm:2622 < callProvider()@resource://gre/modules/AddonManager.jsm:227 < _startProvider()@resource://gre/modules/AddonManager.jsm:755 < AddonManagerInternal.startup()@resource://gre/modules/AddonManager.jsm:938 < this.AddonManagerPrivate.startup()@resource://gre/modules/AddonManager.jsm:2773 < amManager.prototype.observe()@resource://gre/components/addonManager.js:57

1465377475734	addons.xpi-utils	WARN	Could not uninstall invalid item from locked install location
Can we bisect to figure out what caused this to start failing?

While we might not need the storage service any more it is possible that if we change the initialization order we might break someone else.
Flags: needinfo?(dtownsend)
Assignee: nobody → aswan
Whiteboard: triaged
Here's a try run with the reference to Services.storage removed (and the uuid generation changed to the more concise uuid-generator).
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eab3b89c683d

There are a bunch of clearly unrelated intermittent failures, I retriggered some of the non-obvious ones and they all passed on a subsequent run.  In any case, I would think that if this change revealed a startup ordering problem, we would see much more spectacular failure.

Dave, you were hesitant about this when we talked before, what do you think?
Flags: needinfo?(dtownsend)
(In reply to Dave Townsend [:mossop] from comment #10)
> Can we bisect to figure out what caused this to start failing?

Oops, we talked about this on IRC but for posterity, I'm unable to reproduce this myself so I can't bisect.  glazou, any chance you could try an older build and/or do a mozregression?
Flags: needinfo?(daniel)
(In reply to Andrew Swan [:aswan] from comment #11)
> Here's a try run with the reference to Services.storage removed (and the
> uuid generation changed to the more concise uuid-generator).
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=eab3b89c683d
> 
> There are a bunch of clearly unrelated intermittent failures, I retriggered
> some of the non-obvious ones and they all passed on a subsequent run.  In
> any case, I would think that if this change revealed a startup ordering
> problem, we would see much more spectacular failure.
> 
> Dave, you were hesitant about this when we talked before, what do you think?

I think we can go ahead and land on central. Did we want to uplift? The original report is from 46. If so we might need to think of some specific things for QA to focus on here.
Flags: needinfo?(dtownsend)
FWIW, the guilty line is now try/catch'd in BlueGriffon with thousands of users
happy with the result on all platforms. Without that change, installation of
add-ons was failing every time ; with the change in, I got tons of feedback the
whole thing works normally again.
Flags: needinfo?(daniel)
(In reply to Andrew Swan [:aswan] from comment #12)

> Oops, we talked about this on IRC but for posterity, I'm unable to reproduce
> this myself so I can't bisect.  glazou, any chance you could try an older
> build and/or do a mozregression?

Hardly. BlueGriffon 2.0 was for me a major uplift and the regular changes in
the build system don't make it easy for me to regress since I'm not updating
the core on a daily basis...
(In reply to Dave Townsend [:mossop] from comment #13)
> I think we can go ahead and land on central. Did we want to uplift? The
> original report is from 46. If so we might need to think of some specific
> things for QA to focus on here.

Right, both to check that the original problem is fixed and to ensure we're not introducing a regression.  Both of those seem challenging!  Once this lands I can ask for approval for Aurora and Beta, is this worth going all the way to release?  (we are at the beginning of an extended release cycle so 48 going to release is still 7+ weeks away)
Kris, can you review?  Mossop already gave verbal approval in comment 13 but he's not accepting reviews right now.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8761613 [details]
Bug 1277295 Remove obsolete reference to storage service

https://reviewboard.mozilla.org/r/58724/#review55736

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1233
(Diff revision 1)
>    Object.defineProperty(aAddon, "syncGUID", {
>      get: () => {
>        // Generate random GUID used for Sync.
> -      // This was lifted from util.js:makeGUID() from services-sync.
> -      let rng = Cc["@mozilla.org/security/random-generator;1"].
> -        createInstance(Ci.nsIRandomGenerator);
> +      let guid = Cc["@mozilla.org/uuid-generator;1"]
> +          .getService(Ci.nsIUUIDGenerator)
> +          .generateUUID().toString();

s/.toString()/.number/

I assume we're OK with the fact that this is going to produce a GUID with a completely different format from the old code?
Attachment #8761613 - Flags: review?(kmaglione+bmo) → review+
(In reply to Kris Maglione [:kmag] from comment #19)
> Comment on attachment 8761613 [details]
> Bug 1277295 Remove obsolete reference to storage service
> 
> https://reviewboard.mozilla.org/r/58724/#review55736
> 
> ::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1233
> (Diff revision 1)
> >    Object.defineProperty(aAddon, "syncGUID", {
> >      get: () => {
> >        // Generate random GUID used for Sync.
> > -      // This was lifted from util.js:makeGUID() from services-sync.
> > -      let rng = Cc["@mozilla.org/security/random-generator;1"].
> > -        createInstance(Ci.nsIRandomGenerator);
> > +      let guid = Cc["@mozilla.org/uuid-generator;1"]
> > +          .getService(Ci.nsIUUIDGenerator)
> > +          .generateUUID().toString();
> 
> s/.toString()/.number/
> 
> I assume we're OK with the fact that this is going to produce a GUID with a
> completely different format from the old code?

Please have a sync person sign off on this...
mconnor, can you take a look at the change above or suggest somebody who can?
Flags: needinfo?(mconnor)
FWIW, I got a bug report about this happening on Debian with esr45.
No love from mconnor.  gps, it looks like you touched this code most recently even though it was a few years ago.  can you take a look at those changes or suggest a good person to take a look?
Flags: needinfo?(mconnor) → needinfo?(gps)
Comment on attachment 8761613 [details]
Bug 1277295 Remove obsolete reference to storage service

https://reviewboard.mozilla.org/r/58724/#review57454

This should be OK.

Note that there are tests (e.g. toolkit/mozapps/extensions/test/xpcshell/test_syncGUID.js) that verify `syncGUID` is 9+ characters. You may want to rewrite those to assert syncGUID is the expected UUID length.
Attachment #8761613 - Flags: review+
https://reviewboard.mozilla.org/r/58724/#review55736

> s/.toString()/.number/
> 
> I assume we're OK with the fact that this is going to produce a GUID with a completely different format from the old code?

Yes. The SyncGUID is used to uniquely identify an add-on in the install. IIRC it is treated as a black box by Sync.
Flags: needinfo?(gps)
Comment on attachment 8761613 [details]
Bug 1277295 Remove obsolete reference to storage service

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58724/diff/1-2/
Attachment #8761613 - Flags: review+
Thanks for the feedback.  Rebased to updated m-c, updated the test as suggested, and doing one more try run out of caution.
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f93302810d0
Remove obsolete reference to storage service r=gps,kmag
https://hg.mozilla.org/mozilla-central/rev/5f93302810d0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Flags: qe-verify+
I tried to reproduce the initial issue using Firefox 46.0.1 and 45.0 on Ubuntu 14.04 64bit but with no success.
I also installed top 10 of the most used add-ons that required restart and all of them installed successfully and were still active after each restart. Testing was done across platforms (Windows 7 64bit, Mac OS X 10.12 and Ubuntu 14.04 64bit) using Firefox 50.0 RC build.

Carsten Sommer can you please verify that 50.0 RC build works for you since you were able to reproduce this problem in the first place?

Link to build: https://archive.mozilla.org/pub/firefox/candidates/50.0-candidates/build1/
Flags: needinfo?(cs_gon)
Hello Bogdan Maris, yes, the FF 50.0 RC build fixes the problem for me. 

I tried to install the Stylish extension in the current FF 49 three times, and each time it failed with the same error messages, as reported in the initial bug report. Then I tried to install the extension in FF 50 RC three times, and this was successful each time. All tests were done on the same user account and with the same user profile.
Flags: needinfo?(cs_gon)
(In reply to Carsten Sommer from comment #31)
> Hello Bogdan Maris, yes, the FF 50.0 RC build fixes the problem for me. 
> 
> I tried to install the Stylish extension in the current FF 49 three times,
> and each time it failed with the same error messages, as reported in the
> initial bug report. Then I tried to install the extension in FF 50 RC three
> times, and this was successful each time. All tests were done on the same
> user account and with the same user profile.

Thanks for this! I'll mark this as verified then.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: