Open Bug 577953 Opened 13 years ago Updated 4 months ago
Minefield appears to be holding on to preference observers
This looks like a pretty bad leak. I'm still debugging, but I have a printf in nsPrefBranch::RemoveObserver that prints the count of observers in there. Observers do get removed, but the baseline seems to keep rising.
After half an hour or so of browsing, I have about 900 of these hanging out. --DOMWINDOW == 17 (0x12180fd38) [serial = 607] [outer = 0x123e6ba90] [url = http://www.mozilla.org/projects/minefield/] --DOMWINDOW == 16 (0x123e6baf8) [serial = 603] [outer = 0x0] [url = http://www.mozilla.org/projects/minefield/] Removing observers. Count is 968. --DOMWINDOW == 15 (0x1077cdce8) [serial = 600] [outer = 0x1077ce3a0] [url = about:blank] --DOMWINDOW == 14 (0x1077ce408) [serial = 599] [outer = 0x0] [url = chrome://browser/content/browser.xul] Removing observers. Count is 967. Removing observers. Count is 966. Removing observers. Count is 965. Removing observers. Count is 964. Removing observers. Count is 963. Removing observers. Count is 962. Removing observers. Count is 961. Removing observers. Count is 960. Removing observers. Count is 959. Removing observers. Count is 958. Removing observers. Count is 957. --DOMWINDOW == 13 (0x124ea5a38) [serial = 615] [outer = 0x124eec6d0] [url = about:blank]
I do have a sane number of DocShells and DOMWindows, though.
logging the membership of this array right now. I can see some problems by eyeballing it.
I don't think security.csp should be a preference anymore. Don't know what the other two leakers do.
Sounds like someone is not removing observers when they should... In particular, the CSP code in the nsDocument constructor is using AddBoolPrefVarCache, which is only meant to be used for global vars, and only meant to be used once per var. That is NOT what the CSP code is doing. We should probably file separate bugs blocking this one on the separate issues... accessibility.browsewithcaret needs to be a pref. Looks like the only pref observers for it are in nsFocusManager.cpp (added in nsFocusManager::Init and removed in ~nsFocusManager) and in nsTypeAheadFind.cpp (added in Init() and removed in the destructor). The focus manager is a service. Are we leaking typeaheadfind instances for the app lifetime or something? browser.zoom also needs to be a pref. The only observer seems to be in browser-fullZoom.js. It removes in destroy(). Are we failing to call that?
blocking2.0: --- → ?
Filed bug 578182 for the CSP leak.
Adding Neil Deakin regarding accessibility.browsewithcaret observer in nsFocusManager (comment 6).
The typeaheadfind component is created in tabbrowser.xml as well as within each <browser>. Not sure why both are needed. Is the test above for repeated opening of new tabs? That might suggest the latter not being deleted. As an aside, I don't think the typeahead find needs to observe the accessibility.browsewithcaret preference. Retrieving it each time should be sufficient.
That should be a separate bug with a separate discussion, esp. because with e10s pref-getting is about to get a lot more expensive (needs an IPC call if it happens in the content process, for example).
Not going to block on the meta-bug: it looks like the top 3 in your list are not per-tab or per-JSContext, I might block on those specifically.
blocking2.0: ? → -
You need to log in before you can comment on or make changes to this bug.