Closed Bug 764840 Opened 9 years ago Closed 8 years ago

loader.js is leaked on addon disabling

Categories

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

Product:
Add-on SDK Graveyard
Component:
General
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

(Whiteboard: [MemShrink:P3])

Attachments

(1 file, 3 obsolete files)

Pull request 548
165 bytes, text/html
irakli
: review+
Details 
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!
Attached file Pull request 465 (obsolete) — 
Some more details, we are mostly leaking loader.js JSM, but not the laoder instance itself.
Here is a patch to fix this.
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.
Attached file Pull request 473 (obsolete) — 
I had to create a new pull request ...
Assignee: nobody → poirot.alex
Attachment #633170 - Attachment is obsolete: true
Attachment #637194 - Flags: review?(rFobic)
Comment on attachment 637194 [details]
Pull request 473

Please see notes in github before landing.
Attachment #637194 - Flags: review?(rFobic) → review+
Attached file Pull request 533 (obsolete) —
Attachment #637194 - Attachment is obsolete: true
Attachment #654652 - Flags: review?(rFobic)
Whiteboard: [MemShrink:P3]
Comment on attachment 654652 [details]
Pull request 533

Just one more thing.
Attachment #654652 - Flags: review?(rFobic) → review+
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
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.11
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...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached file Pull request 548 —
Attachment #654652 - Attachment is obsolete: true
Attachment #657857 - Flags: review?(rFobic)
Target Milestone: 1.11 → ---
Comment on attachment 657857 [details]
Pull request 548

Please make sure to address comments in pull request before landing.
Attachment #657857 - Flags: review?(rFobic) → review+
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
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.11
You need to log in before you can comment on or make changes to this bug.