Closed Bug 671894 Opened 13 years ago Closed 12 years ago

addons.xpi: Failed to open database (NS_ERROR_STORAGE_BUSY)

Categories

(Toolkit :: Add-ons Manager, defect)

Product:
Toolkit
Component:
Add-ons Manager
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla18
Tracking Flags:
Tracking Status
firefox10 - ---

People

(Reporter: glandium, Assigned: Unfocused)

References

Details

(Keywords: reproducible)

Attachments

(2 files, 3 obsolete files)

Part 1, v2
66.24 KB, patch
Unfocused
: review+
Details | Diff | Splinter Review
Part 2, v1
10.52 KB, patch
mossop
: review+
Details | Diff | Splinter Review 
Going to about:addons was showing "no addons of this type installed" for extensions, while a few extensions related menu items and such were still in the UI. In fact, e.g. DOM inspector was still working fine. But restartless extensions such as lesschrome hd, weren't working.

extensions.log contains this:
2011-07-15 08:08:45 ERROR addons.xpi: Failed to open database (1st attempt): [Exception... "Component returned failure cod
e: 0x80630001 (NS_ERROR_STORAGE_BUSY) [mozIStorageService.openUnsharedDatabase]"  nsresult: "0x80630001 (NS_ERROR_STORAGE_
BUSY)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: XPIDB_openDatabaseFile :: line 4031"  data: no] a
t resource://gre/modules/XPIProvider.jsm:4031
2011-07-15 08:08:45 ERROR addons.xpi: Error processing file changes: TypeError: this.installLocations is null at resource:
//gre/modules/XPIProvider.jsm:1737

(that's unfortunately the only content)

Removing the extensions.sqlite file solved the problem, though all extensions ended up being enabled (unsurprisingly).

That leads to two questions:
- how did this happen? (I have a copy of the failing extensions.sqlite file, if required)
- why the hell use sqlite for *that*?

FWIW, Pike had the same problem today. I'm on aurora (though I'm using system sqlite), he's on nightly.
(In reply to comment #0)
> That leads to two questions:
> - how did this happen? (I have a copy of the failing extensions.sqlite file,
> if required)

Based on the error code appears that some process had the sqlite file already open. For performance (and safety) reasons we only allow one process to open the file at a time. Odd though, we did have code in place that made us work mostly sanely when the DB wasn't accessible.

> - why the hell use sqlite for *that*?

For what? The database of extensions?
FWIW, I might have forcefully quit an 5.0 version accidently running on that profile for nightly. As in, no, I'm not using multiple concurrent instances on the same profile, but it could have been that I used an older version on that profile, and I killed that early, and hard.
(In reply to comment #1)
> (In reply to comment #0)
> > That leads to two questions:
> > - how did this happen? (I have a copy of the failing extensions.sqlite file,
> > if required)
> 
> Based on the error code appears that some process had the sqlite file
> already open. For performance (and safety) reasons we only allow one process
> to open the file at a time. Odd though, we did have code in place that made
> us work mostly sanely when the DB wasn't accessible.

Based on the log time, it would be the first startup in the morning. Nothing else was running at the same time.

> > - why the hell use sqlite for *that*?
> 
> For what? The database of extensions?

Yes, that.
(In reply to comment #2)
> FWIW, I might have forcefully quit an 5.0 version accidently running on that
> profile for nightly. As in, no, I'm not using multiple concurrent instances
> on the same profile, but it could have been that I used an older version on
> that profile, and I killed that early, and hard.

I would hope that wouldn't cause the same problem, but I'll do some tests with it.

(In reply to comment #3)
> (In reply to comment #1)
> > (In reply to comment #0)
> > > That leads to two questions:
> > > - how did this happen? (I have a copy of the failing extensions.sqlite file,
> > > if required)
> > 
> > Based on the error code appears that some process had the sqlite file
> > already open. For performance (and safety) reasons we only allow one process
> > to open the file at a time. Odd though, we did have code in place that made
> > us work mostly sanely when the DB wasn't accessible.
> 
> Based on the log time, it would be the first startup in the morning. Nothing
> else was running at the same time.

Do you have the add-ons compatibility reporter installed at all?

> > > - why the hell use sqlite for *that*?
> > 
> > For what? The database of extensions?
> 
> Yes, that.

We have to maintain it in some kind of datastore. We used to use RDF but saw issues with corruption and code complexity. Barring writing something custom sqlite was chosen to replace it.
(In reply to comment #0)
> 2011-07-15 08:08:45 ERROR addons.xpi: Error processing file changes:
> TypeError: this.installLocations is null at resource:
> //gre/modules/XPIProvider.jsm:1737

Actually this is an odd error. It suggests that something was calling into the add-ons manager late in shutdown.
(In reply to comment #4)
> > Based on the log time, it would be the first startup in the morning. Nothing
> > else was running at the same time.
> 
> Do you have the add-ons compatibility reporter installed at all?

I do.
If i believe the timestamp of the compatibility@addons.mozilla.org.xpi file, it has been upgraded at the time of the error
Even better, it has been modified at the *exact* time of the error:
Modify: 2011-07-15 08:08:45.000000000 +0200
I suspect that this is from bug 572322. The first time after ACR 0.8.6 is installed it will flip the compatibility prefs and then restart the app before the main UI even shows. To the user it just looks like a regular if maybe slightly longer startup. I imagine we're ending up with the first process not fully closing the database before the second process tries to open it.

I've not yet reproduced that for myself yet though, but that could be due to there needing to be something else going on, perhaps another extension installed that tries to access the add-ons manager immediately before/after ACR attempts to restart.

Setting extensions.acr.postinstall to false and restarting may cause the issue to reproduce again, but if this really is the cause then it will probably be very dependent on timing
Blocks: 572322
I have two questions :

1) How did you generate the log ? (is it a nspr flag that needs to be set) ?
2) Does blowing away the sql file regenerates it ?
(In reply to comment #12)
> I have two questions :
> 
> 1) How did you generate the log ? (is it a nspr flag that needs to be set) ?
> 2) Does blowing away the sql file regenerates it ?

Found the answers... ta
I have also tested this and wasn't able to reproduce it myself. Could it be that an user needs to have a lot of extensions installed? Would be great to get the extensions list from everyone who experiences this problem. Then we could check for similarities.
(if only about:addons was selectable...)

Here is my list:
About startup 0.1.5
Add-on Compatibility Reporter 0.8.6
Diggler 0.9
DOM Inspector 2.0.9
Live HTTP Headers 0.17
Mass Password Reset 1.04
Mozilla Labs: Prospector - LessChrome HD 7
Rikaichan 2.03
Rikaichan Japanese-English Dictionary File 2.01.110527
Bugzilla Tweaks 1.10 (disabled)
DownThemAll! 1.1.8 (disabled)
Firebug 1.7.1 (disabled)
FireFontFamily 0.1.1 (disabled)
HttpFox 0.8.9 (disabled)
JavaScript Debugger 0.9.88.2 (disabled)
Page Speed 1.10.2 (disabled)
Tamper Data 10.0.3 (disabled)
Vimperator 1.0 (disabled)
harharhar, about:support support clipboard, true and false is activated or not:

        Add-on Builder Helper
        0.0.19
        true
        jid0-t3eeRQgGANLCH9c50lPqcTDuNng@jetpack

        Add-on Compatibility Reporter
        0.8.6
        true
        compatibility@addons.mozilla.org

        Bugzilla Tweaks
        1.10
        true
        jid0-qBnIpLfDFa4LpdrjhAC6vBqN20Q@jetpack

        Deutsches Wörterbuch
        2.0.2
        true
        de-DE@dictionaries.addons.mozilla.org

        DOM Inspector
        2.0.10
        true
        inspector@mozilla.org

        F1 by Mozilla Labs
        0.8.3
        true
        ffshare@mozilla.org

        Firebug
        1.8.0b5
        true
        firebug@software.joehewitt.com

        Font Information
        0.1
        true
        {70ded480-0a45-4099-84d1-65aa1cb1575e}

        Mass Password Reset
        1.04
        true
        masspasswordreset@johnathan.nightingale

        United States English Spellchecker
        5.0.1
        true
        en-US@dictionaries.addons.mozilla.org

        Bugzilla L10n Component Assistant
        0.1
        false
        bugzilla@l10n.mozilla.org

        JavaScript Debugger
        0.9.88.1
        false
        {f13b157f-b174-47e7-a34d-4815ddfdfeb8}

        MozMill
        1.5.2
        false
        mozmill@mozilla.com

        Mozmill Crowd
        0.1.3
        false
        mozmill-crowd@quality.mozilla.org
here's mine :

        About startup
        0.1.5
        true
        aboutstartup@glandium.org

        about:telemetry
        0.3
        true
        ping.telemetry@mozilla.com

        Add-on Compatibility Reporter
        0.8.6
        true
        compatibility@addons.mozilla.org

        Better Flickr
        0.4.2
        true
        betterflickr@ginatrapani.org

        Conspiracy
        0.3.0
        true
        {5f8164ae-06a4-4a65-9b88-281d8cdcf548}

        Delicious Bookmarks
        2.1.104
        true
        {2fa4ed95-0317-4c6a-a74c-5f3e3912c1f9}

        Dictionnaire français «Classique & Réforme 1990»
        4.1
        true
        fr-classique-reforme1990@dictionaries.addons.mozilla.org

        F1 by Mozilla Labs
        0.8.3
        true
        ffshare@mozilla.org

        Feedback
        1.1.2
        true
        testpilot@labs.mozilla.com

        Ghostery
        2.5.3
        true
        firefox@ghostery.com

        Greasemonkey
        0.9.2
        true
        {e4a8a97b-f2ed-450b-b12d-ee082ba24781}

        HTTPS-Everywhere
        0.9.7
        true
        https-everywhere@eff.org

        Linky
        3.0.0
        true
        linky@gemal.dk

        Woordenboek Nederlands
        3.0.1
        true
        nl-NL@dictionaries.addons.mozilla.org
No longer blocks: 572322
Depends on: 572322
With bug 572322 fixed, this problem should no longer be seen (when using Add-on Compatibility Reporter 1.0).
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a2) Gecko/20111121 Firefox/10.0a2

I stumbled upon the same issue as described in comment 0 while testing Add-ons Default to Compatible, but could not reproduce it after several tries.I used the same profile with two different Firefox builds and after two consecutive restarts the extension and appearance list was empty.

1. Started Firefox 9 Beta2 with clean profile.
2. Installed the 3 add-ons listed below. (Feedback and Ubuntu Firefox-default for Ubuntu)
3. Closed Firefox and started Firefox Aurora (10a2)
4. Opened add-ons Manager and about:config.
5. In about:config switched the extensions.strictCompatibility pref to false
6. Restarted when prompt from Add-on Manager for now compatible add-ons.
7. After restart, enabled Ubuntu Software add-on and restarted when prompted.

After the second restart add-ons were no longer displayed in Add-on manager or about:support. The menu entries for the add-ons installed were available though. 

While trying to investigate further, used the same profile with a third build (Firefox Nightly-11a1) and at start-up every add-on was displayed in a separate tab with the third party warning message.
 
Feedback 1.1.2
FireFox Tweak 2.0
LinkedIn Companion for Firefox 3.5.1
Ubuntu Firefox Modifications 1.0
Grafx Bot 9.0.00

Extensions.log contains the following messages:

2011-11-22 15:03:15 ERROR addons.xpi: Failed to open database (1st attempt): [Exception... "Component returned failure code: 0x80630001 (NS_ERROR_STORAGE_BUSY) [mozIStorageService.openUnsharedDatabase]"  nsresult: "0x80630001 (NS_ERROR_STORAGE_BUSY)"  location: "JS frame :: resource:///modules/XPIProvider.jsm :: XPIDB_openDatabaseFile :: line 4155"  data: no] at resource:///modules/XPIProvider.jsm:4155
2011-11-22 15:03:15 ERROR addons.xpi: Error processing file changes: TypeError: this.installLocations is null at resource:///modules/XPIProvider.jsm:1798
Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0 beta 3

Managed to reproduce this 4 times today when upgrading from Firefox9Beta5 to Firefox 10B3. After trying to isolate, couldn't anymore. Will check into this tomorrow again.

I still the profiles available if any extra information is needed.

1. Start Firefox 9B5.
2. Install Greasemonkey 0.9.13.
3. Play a youtube video.
4. Disable the Greasemonkey add-on before applying the update without restarting Firefox. (the add-on is in pending-Restart needed before the add-on is disabled)
5. Update to 10Beta3 from About:Firefox (on betatest channel).
6. After the update, while the youtube clip is loading, enable the Greasemonkey add-on and restart.

Actual result: Add-on manager did not list any add-ons. 
Extensions.log contains the NS_Storage_Busy_Error message as described in comment 0 and 19.

error console error message and error warning: 

Error: extensions.get(EXTENSION_ID) is null
Source File: resource://testpilot/modules/setup.js
Line: 655 

Warning: WARN addons.manager: Exception calling callback: [Exception... "'[JavaScript Error: "extensions.get(EXTENSION_ID) is null" {file: "resource://testpilot/modules/setup.js" line: 655}]' when calling method: [extIExtensionsCallback::callback]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: resource:///components/fuelApplication.js :: <TOP_LEVEL> :: line 1348"  data: yes]
Source File: resource:///components/fuelApplication.js
Line: 1348
Whiteboard: [10b3]
tracking-firefox10: --- → ?
From my read of this bug there's not reason to believe that this is occurring with any greater frequency than 6 months ago when it was first reported. Therefore this is not a regression. It's great that we have STR, however.

Please re-nominate if I've got that wrong.
tracking-firefox10: ? → -
Keywords: reproducible
Dave - this sounds reproducible. Who's in a position to investigate this?
(In reply to Alex Keybl [:akeybl] from comment #25)
> Dave - this sounds reproducible. Who's in a position to investigate this?

When I tried I couldn't reproduce this, but Blair is probably the man for the job
Thanks Dave!

Blair - can you take a crack at reproducing (https://bugzilla.mozilla.org/show_bug.cgi?id=671894#c21) and investigating when you get the chance?
Assignee: nobody → bmcbride
I couldn't reproduce the issue with the steps in comment 21 with Firefox 10 Beta at its time and neither could now after attempting again.

Here's a summary from duplicates and own experience:
-in all cases changing an add-on state seemed to trigger the issue (enable-disable and restarting, or a background update for an add-on-ACR in the case of this bug report)
-any change in extension.sqlite (installing an add-on, deleting the file) fixed the problem
-not a regression-not a recent one at least (reported since 2011/7/15)
(In reply to Virgil Dicu [:virgil] [QA] from comment #28)
> I couldn't reproduce the issue with the steps in comment 21 with Firefox 10
> Beta at its time

After managing to reproduce 4 times consecutively, that is.
Blocks: 712542
No longer blocks: 712542
Attached patch protect against invalid calls (obsolete) — — Splinter Review 
Been trying to figure out potential problems and solutions here. One thing I've seen in a number of logs is evidence of something attempting to access the add-ons manager after it has been shutdown. I can only guess at three causes for this, a fault in our code, or some add-on either shutting down the add-ons manager itself or calling it during shutdown. Either that or an async operation is waiting to complete when we start shutdown and completes after, trying to use the DB again in the process.

When this happens the DB is already locked and so we attempt to delete it and recreate from scratch to continue, but since the add-ons manager is shutdown most of the variables we need are uninitialised so this fails probably leaving an empty DB in the profile. This is probably why some people see empty add-ons lists until they install a new add-on (causes a complete re-scan of the profile).

This first patch attempts to protect against a lot of this. It removes the currently fairly visible API to shutdown the add-ons manager (it's still possible to do, just harder) and rejects any attempts to use the API while it is shutdown.
Attachment #605560 - Flags: review?(bmcbride)
Attached patch recover correctly during startup (obsolete) — — Splinter Review 
This second patch prevents us from deleting the database in the event that it is only locked on the assumption we'll be able to open it in the future (that might be a bad assumption, not sure). As it happens it's impossible to delete the database on windows in this case anyway so regardless I think I'd like to do this if only to keep all the platforms behaving the same in this already tricky to handle case.

I also discovered why people are often seeing the new-addons prompt for all their add-ons in the case that the DB is locked or corrupt and there are new add-ons installed or removed during startup. In that case we currently do do the correct recovery by loading the list of enabled add-ons from extensions.ini and we just assume that all add-ons have appeared in the profile and are foreign installs. This patch changes it so we do proper recovery even during startup.

This doesn't perfectly fix the problem. If the user installs a new add-on, restarts, gets a locked database, then restarts again with an accessible database it currently shows the new add-on prompt for that installed add-on as we didn't save it to the database. I'm still trying to figure out a sane way to avoid that.
Attachment #605562 - Flags: feedback?(bmcbride)
Comment on attachment 605560 [details] [diff] [review]
protect against invalid calls

Review of attachment 605560 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/AddonLogging.jsm
@@ +166,5 @@
> +    let message = formatLogMessage("stack", this.name, aStr, aException);
> +    dump("*** " + message + " (" + stack.sourceName + ":" + stack.lineNumber +")\n");
> +    Services.console.logStringMessage(message);
> +
> +    stack = Components.stack.caller.caller;

Is this the right stack frame? It looks like the same stack frame as what's dumped above (getStackDetails uses Components.stack.caller.caller.caller if aException is null). Either way, using Components.stack rather then passing around a specific stack frame seems... Inception-worthy (not in a good way). Would be clearer if this at least used the result of getStackDetails (ie, stack.caller).

@@ +168,5 @@
> +    Services.console.logStringMessage(message);
> +
> +    stack = Components.stack.caller.caller;
> +    while (stack) {
> +      dump("*** " + stack.name + " (" + stack.filename + ":" + stack.lineNumber +")\n");

Why not just use stack.toString() ? (explicitly or implicitly)

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +559,5 @@
>                url + "\"", e);
>        }
>      }
>  
> +    // Once we start calling providers we must allow all normal methods to work

Style nit: missing full-stop.

::: toolkit/mozapps/extensions/test/xpcshell/test_shutdown.js
@@ +1,5 @@
> +/* Any copyright is dedicated to the Public Domain.
> + * http://creativecommons.org/publicdomain/zero/1.0/
> + */
> +
> +// Verify that API functions fail if the add-ons manager isn't initialised

Style nit: missing full-stop.

@@ +7,5 @@
> +function test_functions() {
> +  ["getInstallForURL", "getInstallForFile", "getAddonByID", "getAddonBySyncGUID",
> +   "getAddonsByIDs", "getAddonsWithOperationsByTypes", "getAddonsByTypes",
> +   "getAllAddons", "getInstallsByTypes", "getAllInstalls", "isInstallEnabled",
> +   "isInstallAllowed", "installAddonsFromWebpage"].forEach(function(aFunc) {

Can you make this a list of what *not* to check? That way it will catch failures if we add anything in the future, but forget to update this test.
Attachment #605560 - Flags: review?(bmcbride) → review-
Comment on attachment 605560 [details] [diff] [review]
protect against invalid calls

Review of attachment 605560 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/AddonLogging.jsm
@@ +157,5 @@
>        Services.console.logStringMessage(message);
>      }
> +  },
> +
> +  stack: function(aStr, aException) {

Just realised: You don't appear to be actually using this. It's useful though (I'm sick of writing stack dumping code when I need it) - spin off to another bug?
Comment on attachment 605562 [details] [diff] [review]
recover correctly during startup

Review of attachment 605562 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +2833,5 @@
>  
>        // Get the migration data for this install location.
>        let locMigrateData = {};
> +      if (XPIDatabase.migrateData && installLocation.name in XPIDatabase.migrateData)
> +        locMigrateData = XPIDatabase.migrateData[installLocation.name];

Since migrateData isn't a parameter being passed around now, I feel like it should be cleared when we're done with that data.

@@ +2937,5 @@
>  
>      // If the application has changed then check for new distribution add-ons
>      if (aAppChanged !== false &&
>          Prefs.getBoolPref(PREF_INSTALL_DISTRO_ADDONS, true))
>        updateDatabase = this.installDistributionAddons(manifests) | updateDatabase;

Probably want to update this line too.

@@ +4090,5 @@
>    // The nested transaction count
>    transactionCount: 0,
>    // The database file
>    dbfile: FileUtils.getFile(KEY_PROFILEDIR, [FILE_DATABASE], true),
> +  // Migration data loaded from an old version of the database

Style nit: missing full-stop.

@@ +4092,5 @@
>    // The database file
>    dbfile: FileUtils.getFile(KEY_PROFILEDIR, [FILE_DATABASE], true),
> +  // Migration data loaded from an old version of the database
> +  migrateData: null,
> +  // Active add-on directories loaded from extensions.ini and prefs at startup

Style nit: missing full-stop.

@@ +4279,5 @@
> +          ERROR("Failed to open database (2nd attempt)", e);
> +  
> +          // If we have got here there seems to be no way to open the real
> +          // database, instead open a temporary memory database so things will
> +          // work for this session

Style nit: missing full-stop.

@@ +4441,5 @@
>      let migrateData = {};
>  
>      // Migrate data from extensions.rdf
>      let rdffile = FileUtils.getFile(KEY_PROFILEDIR, [FILE_OLD_DATABASE], true);
> +    if (!rdffile.exists())

Does this really need to explicitly call .exists() and do another fstat? FileUtils.getFile() already results in a fstat (eg, see bug 731495). Instead, does gRDF.GetDataSourceBlocking() throw if the file doesn't exist?

To double check: The rest of this function is just indentation changes, yes?
Attachment #605562 - Flags: feedback?(bmcbride) → feedback+
(In reply to Dave Townsend (:Mossop) from comment #32)
> This doesn't perfectly fix the problem. If the user installs a new add-on,
> restarts, gets a locked database, then restarts again with an accessible
> database it currently shows the new add-on prompt for that installed add-on
> as we didn't save it to the database. I'm still trying to figure out a sane
> way to avoid that.

Can it not just compare what's in extensions.ini when it encounters what might be a foreign install?

Although, it seems bug 731489 would fix that rather cleanly.
Assignee: bmcbride → dtownsend+bugmail
Status: NEW → ASSIGNED
Attached patch protect against invalid calls (obsolete) — — Splinter Review 

        Attachment #605560 -
        Attachment is obsolete: true

        Attachment #608484 -
        Flags: review?(bmcbride)

    
    

    
  
(In reply to Blair McBride (:Unfocused) from comment #36)
> (In reply to Dave Townsend (:Mossop) from comment #32)
> > This doesn't perfectly fix the problem. If the user installs a new add-on,
> > restarts, gets a locked database, then restarts again with an accessible
> > database it currently shows the new add-on prompt for that installed add-on
> > as we didn't save it to the database. I'm still trying to figure out a sane
> > way to avoid that.
> 
> Can it not just compare what's in extensions.ini when it encounters what
> might be a foreign install?

We'd have to load extensions.ini to do that, so we'd probably have to set a pref or something saying the last run was with an in-memory DB so we only loaded it when necessary. It also wouldn't work for disabled add-ons but perhaps that is the best we can hope for.

> Although, it seems bug 731489 would fix that rather cleanly.

Actually that'd break it.

    
    

    
  
(In reply to Blair McBride (:Unfocused) from comment #35)
> @@ +4441,5 @@
> >      let migrateData = {};
> >  
> >      // Migrate data from extensions.rdf
> >      let rdffile = FileUtils.getFile(KEY_PROFILEDIR, [FILE_OLD_DATABASE], true);
> > +    if (!rdffile.exists())
> 
> Does this really need to explicitly call .exists() and do another fstat?
> FileUtils.getFile() already results in a fstat (eg, see bug 731495).
> Instead, does gRDF.GetDataSourceBlocking() throw if the file doesn't exist?

This isn't entirely true, getFile stats the parent directory (i.e. the profile) but not extensions.rdf itself. And GetDataSourceBlocking doesn't throw if the file doesn't exist, it just returns an empty RDF datasource. I could attempt to guess if there was no extensions.rdf when there are no extensions listed but that isn't necessarily accurate. As this happens hopefully rarely I think it's better to just take the stat.

> To double check: The rest of this function is just indentation changes, yes?

Yep

    
    

    
  
Comment on attachment 608484 [details] [diff] [review]
protect against invalid calls

Review of attachment 608484 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/AddonLogging.jsm
@@ +163,5 @@
>    getLogger: function(aName, aTarget) {
>      let logger = new AddonLogger(aName);
>  
>      if (aTarget) {
> +      ["error", "warn", "log", "stack"].forEach(function(name) {

Think you forgot to revert this line.

::: toolkit/mozapps/extensions/test/xpcshell/test_shutdown.js
@@ +17,5 @@
> +    if (IGNORE.indexOf(prop) != -1)
> +      continue;
> +
> +    try {
> +      dump(prop + "\n");

Nit: do_print()

        Attachment #608484 -
        Flags: review?(bmcbride) → review+
Blocks: 766248

    
    

    
  
I encountered this again, and thought I might add something I haven't seen in other comments.

Using:
Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/16.0 Firefox/16.0 (16.0a2 2012-07-22)
Possibly Firefox was also upgraded from 2012-07-21 by restarting.

* Firefox crashed for some reason
* I tried to restart, but couldn't because Firefox was "already running"
* I check processes: no Firefox process was active
* I manually delete .parentlock of the profile I use
* I start Firefox, but it warns about addons installed by a third-party

My extensions.log contains something slightly different from what's described in comment 0. The complete contents of extensions.log:

2012-07-22 23:43:06 ERROR addons.xpi-utils: Failed to open database (1st attempt): [Exception... "Component returned failure code: 0x80630001 (NS_ERROR_STORAGE_BUSY) [mozIStorageService.openUnsharedDatabase]"  nsresult: "0x80630001 (NS_ERROR_STORAGE_BUSY)"  location: "JS frame :: resource:///modules/XPIProvider.jsm -> resource://gre/modules/XPIProviderUtils.js :: XPIDB_openDatabaseFile :: line 478"  data: no] at resource:///modules/XPIProvider.jsm -> resource://gre/modules/XPIProviderUtils.js:478

I have the following addons installed (not sure which ones were enabled):

Abduction! 3.0.14
ChatZilla 0.9.88.2
Dictionnaires français 4.5
Dominant Color 2
Enjoy Reading 1.0.1
Feed Sidebar 5.1.2
Firebug 1.9.2
Flashblock 1.5.15.1
Ghostery 2.7.2
HTTPS-Everywhere 2.0.5
InstaRead 0.1
LeechBlock 0.6.3
Stylish 1.2.6
Suspend background tabs 1.0.1
Test Pilot 1.2.1
Ubuntu Firefox Modifications 2.0.3
User Agent RG 1.0
Web Developer 1.1.9
Depends on: 782881

    
    

    
  
Note: I've split of the "protect against invalid calls" patch into bug 782881, since its separate enough for its own bug.
Blocks: 769508
Assignee: dtownsend+bugmail → bmcbride

        Attachment #608484 -
        Attachment is obsolete: true

    
    

    
  
I have run into this as well. Is there a short-term workaround to get add-ons back?

    
    

    
  
(In reply to Brian King (Briks) [:kinger] from comment #46)
> I have run into this as well. Is there a short-term workaround to get
> add-ons back?

Can go into about:config and change the value of extensions.lastAppVersion to anything (alternatively, install any addon). Then restart.

    
    

    
  
(In reply to Blair McBride (:Unfocused) from comment #47)
> Can go into about:config and change the value of extensions.lastAppVersion
> to anything (alternatively, install any addon). Then restart.

An Aurora update fixed it for me, which is essentially the same thing.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Part 1, v2
        — 
          — Splinter Review
      

    


  
This is Dave's original patch (ie, attachment 605562 [details] [diff] [review]), updated long ago by Dave to address my comments. All I've done is remove the bitrot, and fixed some nits. I've gone through this a few times, along with all the related code. This patch gets my r+, though I have some additional minor changes coming up in a second patch, and a crazy idea in a followup bug.

        Attachment #605562 -
        Attachment is obsolete: true

        Attachment #662092 -
        Flags: review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Part 2, v1
        — 
          — Splinter Review
      

    


  

        Attachment #662093 -
        Flags: review?(dtownsend+bugmail)
Depends on: 791987

        Attachment #662093 -
        Flags: review?(dtownsend+bugmail) → review+

    
    

    
  
https://hg.mozilla.org/integration/fx-team/rev/2cef0f510e79
https://hg.mozilla.org/integration/fx-team/rev/d3089bef49ea
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [10b3] → [fixed-in-fx-team]
Target Milestone: --- → mozilla18

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/2cef0f510e79
https://hg.mozilla.org/mozilla-central/rev/d3089bef49ea
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Blocks: 674344

    
    

    
  
It happens for me quite often, how about backporting it to Aurora & Beta?

    
    

    
  
(In reply to Michał Z. Gołębiowski from comment #58)
> It happens for me quite often, how about backporting it to Aurora & Beta?

According to the target milestone this is in current Aurora 18. This is probably not going to backported to beta.

    
    

    
  
Yeah, this landed for 18.0a1 Nightly on Sept 25th so this would have naturally uplifted with the merge on October 8th. Considering we are only a couple of weeks away from the next merge, I'm not sure it's worth uplifting further. I suspect that would carry some risk and we could benefit from those 2 weeks of remaining Aurora & Nightly feedback.

Michal, if you are using the latest mozilla-aurora build this should be fixed. Beta should be fixed in a couple of weeks. If this is not fixed for you in Aurora then please provide details about how you reproduce it so we can either reopen this bug or file a new one.

    
    

    
  
(In reply to Michał Z. Gołębiowski from comment #58)
> It happens for me quite often, how about backporting it to Aurora & Beta?

Sorry, but given the type of changes made here, I'd consider this too risky to backport to Beta. It really needs as much testing as possible, especially considering it's not easily reproducible (except for a few people).

And it's already on current Aurora - if you've seen this bug's symptoms or the error code on there (18), please file a separate bug (and/or make sure it doesn't get duped to this one, but is marked as dependent).

    
    

    
  
So I had such a problem with extensions.sqlite and just updated to Fx 18 b1. Now I get a warning for every addon just as they were installed by another programm (the protection to prevent bad addons shipping with other software to alter Fx). This is confusing if something like this happens to "normal" users and confused me (who knows that there was a problem with my extension database) for the first 2-3 add-ons, too (I know I can enable them in the prefs again). 

Is there a way around this issue/warning at all?

    
    

    
  
Hi,
I have this problem again with firefox-16.0.2:
- the 'get add-ons' page displays 'loading' forever
- extensions shows nothing while I do have several extensions installed
- I tried to clear the extensions.lastAppVersion flags, but when firefox restarts it checks for compatibility 'forever'

I can't try to upgrade to 18.x because my host has an old glibc. I would like another workaround to at least be able to manage my add-ons again.

Does the compatibility check make Internet accesses? (it may have proxy problems)

I tried to use strace, but didn't manage to identify what could cause a problem.

Thanks.

    
    

    
  
(In reply to Christophe Lyon from comment #64)
> I would like another workaround to at least be able to manage my add-ons again.

Delete all extensions.* in your Firefox profile folder. (but leave the extensions subfolder)

    
    

    
  
It didn't work as expected:
- all my extensions were disabled
- but the 'get adds-ons' page still kept being busy
- and the 'extensions' one empty

    
    

    
  
(In reply to Christophe Lyon from comment #66)
Deleting (and thus automatically regenerating) extensions.* but leaving the extensions dir in place does reset the list of extensions. Obvious question: you did do this with Firefox exited; terminated if necessary?

    
    

    
  
(In reply to Dave Garrett from comment #67)
Yes I did quit firefox (cleanly, and checked that no process was still running).

Just did it again: the new extensions.ini has an empty [ExtensionDirs] section.
And extensions.log contains 2 lines of error:
- failed to open database (NS_ERROR_STORAGE_BUSY)
- error creation statement getAllAddons (SELECT .... FROM addon) at null:0

    
    

    
  
Given that this is fixed in the release version, could you please take this to either https://support.mozilla.org/ or IRC? Unfortunately, Bugzilla isn't terribly well suited for this type of discussion.

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: