Last Comment Bug 724433 - "empty" SDK add-on creates zombie compartments in SDK 1.4.2
: "empty" SDK add-on creates zombie compartments in SDK 1.4.2
Status: RESOLVED FIXED
[MemShrink:P2]
: mlk, regression
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: P1 critical with 1 vote (vote)
: ---
Assigned To: Alexandre Poirot [:ochameau] PTO, back on 1st
:
Mentors:
Depends on:
Blocks: LeakyAddons ZombieCompartments 724404 725603 726485
  Show dependency treegraph
 
Reported: 2012-02-05 15:18 PST by Nils Maier [:nmaier]
Modified: 2012-04-17 08:18 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Pull request 339 (165 bytes, text/html)
2012-02-08 07:01 PST, Alexandre Poirot [:ochameau] PTO, back on 1st
rFobic: review+
Details

Description Nils Maier [:nmaier] 2012-02-05 15:18:24 PST
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) 2012-02-06 07:52:04 PST
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] 2012-02-06 10:26:50 PST
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] PTO, back on 1st 2012-02-08 07:01:29 PST
Created attachment 595400 [details]
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) 2012-02-12 16:12:19 PST
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) 2012-02-13 04:28:43 PST
An issue which should be considered as blocker for 1.5?
Comment 6 Dave Townsend [:mossop] 2012-02-13 10:32:01 PST
(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] 2012-02-13 10:38:16 PST
(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] 2012-02-13 11:44:27 PST
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) 2012-02-13 16:13:56 PST
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] 2012-04-17 08:18:11 PDT
Sorry reopend wrong bug.

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