Closed
Bug 557477
Opened 15 years ago
Closed 15 years ago
Content/Chrome tabs leak javascript objects/functions
Categories
(Thunderbird :: Toolbars and Tabs, defect)
Thunderbird
Toolbars and Tabs
Tracking
(thunderbird3.1 beta2-fixed, thunderbird3.0 .5-fixed)
RESOLVED
FIXED
Thunderbird 3.1b2
Tracking | Status | |
---|---|---|
thunderbird3.1 | --- | beta2-fixed |
thunderbird3.0 | --- | .5-fixed |
People
(Reporter: standard8, Assigned: standard8)
Details
(Keywords: memory-leak)
Attachments
(1 file)
2.08 KB,
patch
|
asuth
:
review+
standard8
:
approval-thunderbird3.0.5+
|
Details | Diff | Splinter Review |
I've just found that leak monitor was updated last year (having given up as it wasn't updated for absolutely ages).
As a result, it shows that we're leaking various things in the main window. Most of these aren't to do with content tabs and need other bugs filing, but I'll do that later.
STR:
1) Install Leak monitor
2) Run Thunderbird
3) Open the what's new page (or migration assistant for chrome tabs)
4) Close the tab
5) Close the main window (if you're not on Mac you may want to open the Address Book first to prevent shut-down).
6) Wait for leak monitor windows to appear.
Reported leaks:
Leaks in window 0x158ee840:
[ ] [leaked object] (1e5c1860, chrome://global/content/bindings/browser.xml, 481-498) = [Function]
[ ] [leaked object] (1e27f440, chrome://messenger/content/threadPane.js, 42-90) = [Function]
[+] [leaked object] (18ab4120) = [XULElement]
[+] [leaked object] (215aea80) = [XULElement]
[ ] [leaked object] (1dede860, chrome://messenger/content/msgMail3PaneWindow.js, 936-947) = [Function]
[ ] [leaked object] (1e5c18a0, chrome://global/content/bindings/browser.xml, 515-531) = [Function]
[ ] [leaked object] (158ee840) = [ChromeWindow]
[+] [leaked object] (1dedbbe0) = [Object]
[-] [leaked object] (18ab41c0) = [XULElement]
[ ] [leaked object] (1e5c1060, chrome://global/content/charsetOverlay.js, 237-245) = [Function]
[+] [leaked object] (1f02a520) = [Object]
576-581) = [Function]
[ ] [leaked object] (196be9e0, chrome://messenger/content/tabmail.xml, 1810-1818) = [Function]
[-] [leaked object] (18ab4220) = [XULElement]
[-] [leaked object] (18ab4240) = [XULElement]
[+] [leaked object] (18ab4260) = [Object]
[+] [leaked object] (1dedb600) = [Object]
[ ] _kAboutRightsVersion = 1
[ ] _protocolSvc = true
[ ] openSpecialTabsOnStartup (1dedb640, chrome://messenger/content/specialTabs.js, 147-176) = [Function]
[+] contentTabType (1dedb660) = [Object]
...
[ ] [leaked object] (20852680, chrome://messenger/content/msgMail3PaneWindow.js, 785-791) = [Function]
[+] [leaked object] (18ab4140) = [XULElement]
[+] [leaked object] (18ab4100) = [XULElement]
[+] [leaked object] (1ea4a900) = [XULElement]
[ ] [leaked object] (1e5c1840, chrome://global/content/bindings/browser.xml, 456-474) = [Function]
[+] [leaked object] (215aea20) = [XULElement]
For this bug, I'm looking specifically at the browser.xml functions and the specialTabs object.
The specialTabs object leak should be once per a main window. The browser.xml functions will be per content/chrome tab opened.
Thankfully, looking at the Firefox specific source code, it reveals that their tabbrowser calls browser.destroy() when closing a tab - this ensures the browser binding cleans up correctly as due to the wonders of xbl, it doesn't always.
So a couple of calls to destroy() fixes the browser.xml leaks. Adding a call to remove the mail-startup-done listener once it has been fired resolves the specialTabs object leak.
I don't think we can explicitly test for this at the moment as we don't have a harness suitable for this type of leak test.
Attachment #437265 -
Flags: review?(bugmail)
Assignee | ||
Updated•15 years ago
|
status-thunderbird3.0:
--- → wanted
Comment 1•15 years ago
|
||
Comment on attachment 437265 [details] [diff] [review]
The fix
We should be able to test for a leak no longer happening by explicitly obtaining a weak reference to the object in question, forcing a cycle-collecting GC, and then verifying that the weak reference is now empty.
Having said that, I agree that the proper solution is a more systemic memory leak detector harness like integrating the leak detector extension, not a bunch of one-off piecemeal tests. (At least not unless we keep regressing the exact same leak before we have the proper solution...)
Attachment #437265 -
Flags: review?(bugmail) → review+
Assignee | ||
Comment 2•15 years ago
|
||
I've raised bug 557764 to investigate the global nature of this. I agree that we don't really want one-off piecemeal tests, especially when it comes to functions or xul elements being held onto.
Assignee | ||
Comment 3•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
status-thunderbird3.1:
--- → beta2-fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1b2
Assignee | ||
Comment 4•15 years ago
|
||
Comment on attachment 437265 [details] [diff] [review]
The fix
Although this is only a small leak, I'm still going to consider it for the 3.0 branch.
Attachment #437265 -
Flags: approval-thunderbird3.0.5?
Assignee | ||
Comment 5•15 years ago
|
||
Comment on attachment 437265 [details] [diff] [review]
The fix
Not had any issues raised about this, and its a minor improvement for users, therefore taking for 3.0.5.
Attachment #437265 -
Flags: approval-thunderbird3.0.5? → approval-thunderbird3.0.5+
Assignee | ||
Comment 6•15 years ago
|
||
I checked in a reduced version of this to branch:
http://hg.mozilla.org/releases/comm-1.9.1/rev/3cb9509da408
The mail-startup-done registration wasn't present there as we didn't have the xpinstall notifications like we do on TB 3.1.
You need to log in
before you can comment on or make changes to this bug.
Description
•