ExtensionManager:_finishOperations removeDirRecursive failed because directory was in use

VERIFIED FIXED in mozilla7

Status

()

Toolkit
Add-ons Manager
--
minor
VERIFIED FIXED
10 years ago
7 years ago

People

(Reporter: timeless, Assigned: mossop)

Tracking

unspecified
mozilla7
x86
Windows XP
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
as defined here, this problem is probably windows only. but it is probably possible to have other platforms which would encounter it (especially fat on linux or various network file systems).

steps:
1. install firefox 3.0
Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
(do it today, be counted!)
2. install noscript 1.6.9.3
3. quit
4. find your profile directory
5. delete extensions.rdf
6. in cmd:
cd extensions\{73a6fe31-595d-460b-a920-fcc0f8843232}
[do not quit this command prompt! and do not change the working directory!]
7. change install.rdf to say 1.6 instead of 1.6.9.3
8. run firefox
9. open addons
10. check for updates
11. install the noscript update
12. click restart firefox
13. open addons
14. check to see if noscript is listed as updated.

2008-06-17 22:21:40 - safeInstallOperation: failed to clean up item location after its contents were properly backed up. Failed to clean up: Mozilla\Firefox\Profiles\Test\extensions\{73a6fe31-595d-460b-a920-fcc0f8843232} ... rolling back file moves and aborting installation.
2008-06-17 22:21:40 - ExtensionManager:_finishOperations - failure, catching exception - lineno: 543 - file: undefined - [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.remove]"  nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  location: "JS frame :: file:///C:/Program%20Files/Mozilla%20Firefox%203/components/nsExtensionManager.js :: removeDirRecursive :: line 543"  data: no]
This is due to the way Windows prevents deleting files and directories under certain circumstances such as the one in your steps with cmd.exe and cd'ing into the extensions directory. I've seen other Windows programs in the past not handle deleting its files / directories when a separate program is using them and I personally think the current behavior is fine since these files and directories are application managed.
(Reporter)

Comment 2

10 years ago
careful. the content indexer and virus scanners both will randomly open these directories. so will things like picasa.

i'd prefer a user facing message (after trying one extra time?).
No problem... I'm for handling edgecases such as this but I'd prefer steps to reproduce that aren't a case of shooting oneself in the foot such as using cmd.exe.
Product: Firefox → Toolkit
Dave, is that issue still valid after the rewrite of the EM? Would we still encounter an issue like that?
(Assignee)

Comment 5

7 years ago
Still a small issue here, if an add-on is getting uninstalled and that fails because it is locked it remains running on next startup, claims to be pending uninstall but will never go away.
Assignee: nobody → dtownsend
(Assignee)

Comment 6

7 years ago
Created attachment 540869 [details] [diff] [review]
patch rev 1

I realised that some of the behaviour here wasn't quite what a user probably wants. If an uninstall/upgrade/install fails because of a failure to copy files then just throwing away that operation probably isn't right. Instead after the restart the operation should stay pending and try again after a restart (or the user should be able to undo it). This is a precursor to actually being able to signal to the user that something went wrong here too but that is something for another bug.

This gets slightly complicated because for the upgrade/install case we need to recreate the pending AddonInstall info out of the add-on waiting to be installed. To do that I moved around some of the initialisation code for AddonInstall to make it hopefully a bit clearer for the different ways it can be initialised.
Attachment #540869 - Flags: review?(robert.bugzilla)
Comment on attachment 540869 [details] [diff] [review]
patch rev 1

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>...
>@@ -5293,45 +5314,34 @@ function getHashStringForCrypto(aCrypto)
>   let binary = aCrypto.finish(false);
>   return [toHexString(binary.charCodeAt(i)) for (i in binary)].join("").toLowerCase()
> }
> 
> /**
>  * Instantiates an AddonInstall and passes the new object to a callback when
>  * it is complete.
callback is no longer applicable

>  *
>- * @param  aCallback
>- *         The callback to pass the AddonInstall to
>...
>@@ -6377,18 +6451,24 @@ AddonInstall.createInstall = function(aC
>  *         A version for the add-on
>  * @param  aLoadGroup
>  *         An nsILoadGroup to associate the download with
>  */
> AddonInstall.createDownload = function(aCallback, aUri, aHash, aName, aIconURL,
>                                        aVersion, aLoadGroup) {
>   let location = XPIProvider.installLocationsByName[KEY_APP_PROFILE];
>   let url = NetUtil.newURI(aUri);
>-  new AddonInstall(aCallback, location, url, aHash, aName, null,
>-                   aIconURL, aVersion, null, null, aLoadGroup);
>+
>+  let install = new AddonInstall(location, url, aHash, null, null, aLoadGroup);
>+  if (url instanceof Ci.nsIFileURL) {
>+    install.initLocalInstall(aCallback);
>+  }
>+  else {
>+    install.initAvailableDownload(aName, null, aIconURL, aVersion, aCallback);
>+  }
nit: brackets not needed and not your normal style

>@@ -6401,19 +6481,26 @@ AddonInstall.createUpdate = function(aCa
>   let releaseNotesURI = null;
>   try {
>     if (aUpdate.updateInfoURL)
>       releaseNotesURI = NetUtil.newURI(escapeAddonURI(aAddon, aUpdate.updateInfoURL));
>   }
>   catch (e) {
>     // If the releaseNotesURI cannot be parsed then just ignore it.
>   }
>-  new AddonInstall(aCallback, aAddon._installLocation, url, aUpdate.updateHash,
>-                   aAddon.selectedLocale.name, aAddon.type,
>-                   aAddon.iconURL, aUpdate.version, releaseNotesURI, aAddon);
>+
>+  let install = new AddonInstall(aAddon._installLocation, url,
>+                                 aUpdate.updateHash, releaseNotesURI, aAddon);
>+  if (url instanceof Ci.nsIFileURL) {
>+    install.initLocalInstall(aCallback);
>+  }
>+  else {
>+    install.initAvailableDownload(aAddon.selectedLocale.name, aAddon.type,
>+                                  aAddon.iconURL, aUpdate.version, aCallback);
>+  }
> };
> 
> /**
>  * Creates a wrapper for an AddonInstall that only exposes the public API
>  *
>  * @param  install
>  *         The AddonInstall to create a wrapper for
>  */
>@@ -7091,20 +7178,22 @@ function AddonWrapper(aAddon) {
>   this.__defineGetter__("pendingOperations", function() {
>     let pending = 0;
>     if (!(aAddon instanceof DBAddonInternal)) {
>       // Add-on is pending install if there is no associated install (shouldn't
>       // happen here) or if the install is in the process of or has successfully
>       // completed the install.
>       if (!aAddon._install || aAddon._install.state == AddonManager.STATE_INSTALLING ||
>           aAddon._install.state == AddonManager.STATE_INSTALLED)
>-        pending |= AddonManager.PENDING_INSTALL;
>+        return AddonManager.PENDING_INSTALL;
>     }
>     else if (aAddon.pendingUninstall) {
>-      pending |= AddonManager.PENDING_UNINSTALL;
>+      // If an add-on is pending uninstall then we ignore any other pending
>+      // operations
>+      return AddonManager.PENDING_UNINSTALL;
Similar comment applies to returning AddonManager.PENDING_INSTALL above
Attachment #540869 - Flags: review?(robert.bugzilla) → review+
(Assignee)

Comment 8

7 years ago
Landed: http://hg.mozilla.org/mozilla-central/rev/aa6e6d48e96a
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Verified fixed with Mozilla/5.0 (Windows NT 5.1; rv:7.0a2) Gecko/20110810 Firefox/7.0a2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.