Closed Bug 719185 Opened 8 years ago Closed Last year

Restartless extension gets disabled on update if trash directory cannot get removed

Categories

(Toolkit :: Add-ons Manager, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED INACTIVE

People

(Reporter: gaubugzilla, Unassigned)

References

Details

Attachments

(2 files)

This is related to bug 719180, an issue that locks XPI files and prevents the add-on manager from removing them. Steps to reproduce:

1. Install the attached extension (no browser restart). The extension locks its XPI file due to bug 719180, it can no longer be removed.
2. Install it again (first update). Observe NS_ERROR_FILE_ACCESS_DENIED message in Error Console and the corresponding warning: "Failed to remove trash directory when installing testtest@adblockplus.org"
3. Install it again (second update). Observe another NS_ERROR_FILE_ACCESS_DENIED message in Error Console and the corresponding warning.

Expected results:

The second warning again says "Failed to remove trash directory when installing testtest@adblockplus.org" but the extension updates correctly. Also possible: "Failed to install" warning and the corresponding "Extension cannot be installed because Nightly cannot modify the needed file" message to the user - the old version of the add-on stays active however.

Actual results (20120118 trunk nightly on Windows 7 x64):

"Failed to install" warning in the error console and a visible "Extension cannot be installed because Nightly cannot modify the needed file" message, the old extension version is no longer active and Add-ons Manager shows that it has an error.

I didn't check but the update sequence appears to be the following:

1. Currently installed extension version is shut down.
2. Old trash directory is removed.
3. New trash directory is created, extension's XPI file is moved there.
4. New XPI file is installed.
5. New extension version is started.
6. trash directory is removed.

The first update fails at step 6 and stays without consequences. Second update fails at step 2 however - after the current extension version has been shut down but before the new one is installed. The simplest solution is probably doing step 2 before step 1 - fail early if the trash directory cannot be removed, leave the currently installed version active. The more complicated solution would be using unique names for files moved to the trash directory - this would ensure that a failure to remove the trash directory isn't critical, there will be no name conflict with the new files moved there.
I'm facing the same bug. I'm writing an addon that automatically updates locales of a given addon. In order to do that, I'm using AddonManager API to automatically uninstall and install an addon.
And it appears to be failing for all jetpack addons.
Here is a fork of Wladimir addon that highlights the same issue but with a minimized test case with a simple file reading operation:
function startup(params, reason)
{
  let uri = params.resourceURI.spec + "test.xml";
  
  let ioservice = Cc["@mozilla.org/networl/io-service;1"].getService(Ci.nsIIOService);
  let channel = ioservice.newChannel(uri, "UTF-8", null);
  let stream = channel.open();
  ...
  stream.close();
}

For some reason, it fails if I use:
  AddonManager.getInstallForFile(file, function(install) {
    install.install();
  });
to update this addon, even if I uninstall it with:
  AddonManager.getAddonByID(addonId, function (addon) {
    addon.uninstall();
  });
But it works fine if I update it manually, with a drag'n drop of the xpi file.
Acutally, with some more investigation I found a wordaround and a some explanations.
This error si obviously due to some file been locked.
So that, everything works fine (for what I tried to achieve!), if I do:

  Components.utils.forceGC();

Before installing the addon. It looks like some unreferenced nsIFile instances are kept around and are locking files (at least on windows). We are lucky that all these instances aren't referenced anywhere, so that we just have to ensure collecting them in order to free locks!

Wladimir, Can you give a try by calling forceGC before doing any install?
It may help to add this call in addon manager code!
I know that in particular you'll get locks if you enumerate nsIFile.directoryEntries and don't close the enumerator you get back first. Likewise of course if you have a file stream open and don't close it properly. GC would probably close both of those things assuming you have no references left.
See Also: → tb-ltng-updateprob
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.