1371363 - DirectoryInstallLocation.uninstallAddon() races with _readAddons()

Status: RESOLVED WORKSFORME

(Toolkit :: Add-ons Manager, defect, P2)

Description

[Andrew Swan [:aswan]]

+++ This bug was initially created as a clone of Bug #1367823 +++

From https://bugzilla.mozilla.org/show_bug.cgi?id=1367823#c19
When telemetry experiments are starting up, they attempt to uninstall any active but unknown experiments.  But if we get into DirectoryInstallLocation.uninstallAddon() before DirectoryInstallLocation._readAddons() has finished, _IDToFileMap is not populated and so the actual xpi never gets removed.  The first manifestation of this that we've encountered is bug 1367823. Over in that bug, the experiments code will defer uninstalling unknown experiments (which is a good thing to do anyway for startup performance) but we should address this footgun in the addons manager.

Comment 1

[u279076]

FYI, the problem I was facing in bug 1367246 comment 3 related to this bug has just been shown to me in the wild on one of my officemates' computers. He updated his Nightly and saw the prompt to enable the experiment. I don't think there's anything I can do now but this may impact the number of users getting my experiment and may be "scary" to some users.

Comment 2

[Liz Henry (:lizzard) (relman/hg->git project)]

This does look confusing. If it's something we can resolve before users on beta or especially, on release would ever run into it, that would be best. So far, I think it's only happened on nightly.  Tracking for 56.

Comment 3

[Liz Henry (:lizzard) (relman/hg->git project)]

Is this still an issue for 56? Or only a problem on nightly?

Comment 4

[Robert Helmer [:rhelmer]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #3)
> Is this still an issue for 56? Or only a problem on nightly?

This is still a problem, from comment 0 my understanding is that it's fairly easy to workaround, more a design problem in our API making it a footgun.

I'm not sure why it blocks bug 1394903, I'll see if there's an easy workaround. If not let's just fix this bug.

Comment 5

[Gerry Chang [:gchang]]

Mark 56 won't fix as we are only 1 week from RC.

Comment 6

[Rob Wu [:robwu]]

DirectoryInstallLocation and _readAddons don't exist any more.

Comment 7

[Rob Wu [:robwu]]

The objects/methods have been renamed in the refactor from https://hg.mozilla.org/mozilla-central/rev/4f4afd7b0c01bff41478c859ecb124d735338562 :

• DirectoryInstallLocation -> DirectoryLocation
• _readAddons -> readAddons

_IDToFileMap is gone, and the removal of it allows uninstallAddon to work as intended: https://hg.mozilla.org/mozilla-central/rev/4f4afd7b0c01bff41478c859ecb124d735338562#l2.713