Last Comment Bug 764840 - loader.js is leaked on addon disabling
: loader.js is leaked on addon disabling
Status: RESOLVED FIXED
[MemShrink:P3]
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: 1.11
Assigned To: Alexandre Poirot [:ochameau]
:
Mentors:
: 729423 (view as bug list)
Depends on:
Blocks: LeakyAddons ZombieCompartments
  Show dependency treegraph
 
Reported: 2012-06-14 08:39 PDT by Alexandre Poirot [:ochameau]
Modified: 2012-09-18 06:23 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Pull request 465 (165 bytes, text/html)
2012-06-14 10:01 PDT, Alexandre Poirot [:ochameau]
rFobic: review-
Details
Pull request 473 (165 bytes, text/html)
2012-06-27 11:30 PDT, Alexandre Poirot [:ochameau]
rFobic: review+
Details
Pull request 533 (165 bytes, text/html)
2012-08-23 09:12 PDT, Alexandre Poirot [:ochameau]
rFobic: review+
Details
Pull request 548 (165 bytes, text/html)
2012-09-03 07:38 PDT, Alexandre Poirot [:ochameau]
rFobic: review+
Details

Description Alexandre Poirot [:ochameau] 2012-06-14 08:39:26 PDT
On current master, loader.js is leaked when we are disabling the addon.
It appeared to me when I've done a memory debugging session on nightly,
which now displays different compartements for system principals.
So that we can see JSM which are leaked!
Comment 1 Alexandre Poirot [:ochameau] 2012-06-14 10:01:10 PDT
Created attachment 633170 [details]
Pull request 465

Some more details, we are mostly leaking loader.js JSM, but not the laoder instance itself.
Here is a patch to fix this.
Comment 2 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2012-06-14 18:56:48 PDT
I was assuming that modules loaded by other modules are unloaded if the loader is unloaded. With that assumption following should have caused unload of loader.

https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/app-extension/bootstrap.js#L195

It looks like that's not the case and we should unload such modules manually. In that case I think we should do it either from bootstrap on `shutdown` or immediately after import. Unloading it on first loader unload introduces inconsistent behavior if more than one loader is created.
Comment 3 Alexandre Poirot [:ochameau] 2012-06-27 11:30:14 PDT
Created attachment 637194 [details]
Pull request 473

I had to create a new pull request ...
Comment 4 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2012-06-27 15:07:57 PDT
Comment on attachment 637194 [details]
Pull request 473

Please see notes in github before landing.
Comment 5 Alexandre Poirot [:ochameau] 2012-08-23 09:12:00 PDT
Created attachment 654652 [details]
Pull request 533
Comment 6 Wes Kocher (:KWierso) 2012-08-30 00:09:21 PDT
*** Bug 729423 has been marked as a duplicate of this bug. ***
Comment 7 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2012-08-31 05:20:39 PDT
Comment on attachment 654652 [details]
Pull request 533

Just one more thing.
Comment 8 [github robot] 2012-09-01 14:01:52 PDT
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/149a2d147a5b306a6f15d70ffc061081c8d3b0bd
Bug 764840: Fix loader leak by using Sandboxes and nuking them instead of JSM.

https://github.com/mozilla/addon-sdk/commit/274ed5582b503e27b26afbacfa661bc176b65fc1
Merge pull request #533 from ochameau/bug/764840-use-sandboxes-for-loader

Bug 764840: Fix loader leak by using Sandboxes and nuking them instead of JSM. r=@gozala
Comment 9 Alexandre Poirot [:ochameau] 2012-09-02 01:41:25 PDT
Oops. Had to back it out as it conflicted with recent ChromeWorker patch.

https://github.com/mozilla/addon-sdk/commit/7bafac0ef200a7a6647840cede7b823b1cedcf3b

`ChromeWorker` isn't injexted in sandboxes, so that loader can't expose it anymore...
Comment 10 Alexandre Poirot [:ochameau] 2012-09-03 07:38:54 PDT
Created attachment 657857 [details]
Pull request 548
Comment 11 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2012-09-07 11:05:59 PDT
Comment on attachment 657857 [details]
Pull request 548

Please make sure to address comments in pull request before landing.
Comment 12 [github robot] 2012-09-18 06:21:42 PDT
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/d31175e94cc43804ded0e6ecacaac356a1779160
Bug 764840 - Fix loader leak by using Sandboxes and nuking them instead of JSM.

https://github.com/mozilla/addon-sdk/commit/02bf831b06f331a301661126f619cc2e5aeddb18
Merge pull request #548 from ochameau/bug/764840

Bug 764840 - Fix loader leak by using Sandboxes and nuking them instead of JSM. r=@gozala

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