724433 - "empty" SDK add-on creates zombie compartments in SDK 1.4.2

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Nils Maier [:nmaier]]

STR:
- Create a new add-on with the following main.js (and nothing else):
require("panel");
- cfx xpi
- Install xpi in a fresh profile
- Disable add-on again
- Go about:memory?verbose
- Minimize memory usage
- Wait a couple of minutes, just to be sure
- Minimize memory usage
-> Observe that all the jetpack compartments are still present

Tested and affected SDK version: 1.4.2
Tested and NOT affected SDK version: 1.3
Tested with the latest Nightly Win64 and official Firefox 10 Win32

Might be related to bug 724404

Comment 1

[Justin Lebar (not reading bugmail)]

Does this bug mean only that an extension's memory is not released when it's disabled?  Are there any other ill effects?

If it's only the former, it's something that should be fixed, but doesn't seem like a huge deal.

Comment 2

[Nils Maier [:nmaier]]

I tested a bit further and found out that it doesn't seem to be a problem with panel, actually...
Using a completely empty main.js will still leak all the compartments from the add-on initialization.

New STR:
- Create a new add-on with a completely empty main.js (and nothing else):
- cfx xpi
- Install xpi in a fresh profile
- Disable add-on again
- Go about:memory?verbose
- Minimize memory usage
- Wait a couple of minutes, just to be sure
- Minimize memory usage
-> Observe that all the jetpack compartments are still present

(In reply to Justin Lebar [:jlebar] from comment #1)
> Does this bug mean only that an extension's memory is not released when it's
> disabled?  Are there any other ill effects?

Don't know if there are any other ill-effects, but I'd guess it's possible.
E.g. - and this is just a guess - observers might still be around and notified of stuff they shouldn't, thus carrying out operations they shouldn't or do carry out the same action twice if the add-on is reloaded. If this is the case, then there might even be some performance penalty, other than garbage collection taking longer of course.

> If it's only the former, it's something that should be fixed, but doesn't
> seem like a huge deal.

Might turn out not to be a huge deal, but it is nasty anyway. Consider very long running instances, e.g. due to hibernation, and add-ons that are disabled and enabled a couple of times in between e.g. when updated. This can keep a lot of stuff around.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 339

Ouch!

Thanks for having taken time to test the SDK, we totally missed that scenario and we are leaking almost everything when addons are disabled :/
Here is a first patch that fixes leaks for an empty addon.
It still leaks when Widget module is used, we will need additional work on top of this one in bug 724404.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

With this patch applied to master we are still leaking the main.js compartment:

161,864 B (00.28%) -- compartment([System Principal], resource://jid1-t81mmgf4fgbddq-at-jetpack/test/lib/main.js, 0x113045000)

Tested by creating a new add-on with an empty main.js and deactivated/uninstalled in Firefox followed by a reduce memory request.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

An issue which should be considered as blocker for 1.5?

Comment 6

[Dave Townsend [:mossop]]

(In reply to Henrik Skupin (:whimboo) from comment #5)
> An issue which should be considered as blocker for 1.5?

I don't think I'd block 1.5 on this right now however if the patch lands today (Irakli is going to take a look at that) then we'll look into whether cherry-picking it for the last RC tomorrow is a good idea or not.

Comment 7

[Kris Maglione [:kmag]]

(In reply to Henrik Skupin (:whimboo) from comment #5)
> An issue which should be considered as blocker for 1.5?

It may be worth noting that our current AMO policies would deny full review to an add-on with these issues if they were not SDK issues that the add-on cannot circumvent.

Comment 8

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/46fe1cefd1d8f84862d1bd1fbb3103a8189f6895
Merge pull request #339 from ochameau/fix-addon-disabling-leaks

fix Bug 724433 - Fix various memory leaks when addon is disabled. r=@ochameau

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This fix looks great and removes all the compartments whenever the extension gets disabled or uninstalled. Nothing is leaked anymore by an empty SDK add-on.

Comment 10

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Sorry reopend wrong bug.