Closed Bug 758894 Opened 12 years ago Closed 12 years ago

'Highlight all' feature causes zombie compartments

Categories

(Toolkit :: Find Toolbar, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: josef, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file)

Steps to reproduce: 1. Visit http://www.mozilla.org/en-US/ 2. Press CTRL+F to raise the search bar. 3. Type 'a' to the search field. 4. Click on 'Highlight all'. 5. Close the tab. Even after refreshing about:compartments several times and waiting approx. 10 minutes, a compartment labeled 'http://www.mozilla.org/en-US/' was still present. about:memory says: │ ├─────1,774,144 B (01.23%) -- compartment(http://www.mozilla.org/en-US/) │ │ ├──1,085,440 B (00.75%) -- gc-heap │ │ │ ├────328,864 B (00.23%) -- objects │ │ │ │ ├──245,728 B (00.17%) ── function │ │ │ │ └───83,136 B (00.06%) ── non-function │ │ │ ├────321,968 B (00.22%) -- arena │ │ │ │ ├──304,760 B (00.21%) ── unused │ │ │ │ ├────8,728 B (00.01%) ── padding │ │ │ │ └────8,480 B (00.01%) ── headers │ │ │ ├────305,232 B (00.21%) -- shapes │ │ │ │ ├──199,200 B (00.14%) ── tree │ │ │ │ ├───53,872 B (00.04%) ── base │ │ │ │ └───52,160 B (00.04%) ── dict │ │ │ ├────121,440 B (00.08%) ── scripts │ │ │ └──────7,936 B (00.01%) ── sundries │ │ ├────320,384 B (00.22%) ── script-data │ │ ├────295,840 B (00.20%) -- shapes-extra │ │ │ ├──205,408 B (00.14%) ── tree-tables │ │ │ ├───47,104 B (00.03%) ── compartment-tables │ │ │ ├───22,112 B (00.02%) ── dict-tables │ │ │ └───21,216 B (00.01%) ── tree-shape-kids │ │ ├─────60,288 B (00.04%) ── objects/slots │ │ ├──────8,672 B (00.01%) ── type-inference/script-main │ │ └──────3,520 B (00.00%) ── other-sundries These results are from Safe Mode. When NoScript is used, two compartments instead of just one get leaked, according to about:compartments. It seems zombie compartments aren't created while performing steps 2-5 above on the 'about:' page, but they are created on 'about:support'. If the Highlight all feature is used in another tab, the old tab's zombie compartment disappears.
Whiteboard: [MemShrink]
I do see the zombie in about:compartments after following the steps outlined. Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120526084732
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: 15 Branch → Trunk
Aryeh, we suspect selection code might be involved here, can you take a look?
Whiteboard: [MemShrink] → [MemShrink:P2]
The highlighter code stores nsIEditors for each of the editable elements that it highlights for use in removing the highlighting. Chances are, these are holding the document alive from binary code since they don't use proxies.
I'm probably not the best person to look at this -- I honestly don't have more than the foggiest understanding of how our garbage collection works. If someone wants to step me through how to deal with this sort of thing, maybe by pointing to some similar bugs whose fixes I can look it, I could try taking a look when I have the time.
> I honestly don't have more than the foggiest understanding of how our garbage collection works Thankfully you don't need to have much of an idea. :) This bug is happening because somehow, someone is holding a strong reference (e.g. nsCOMPtr, nsRefPtr) to some object which transitively holds a strong reference to a window object. It could be a DOM node, an event, or any number of things. See for example Roc's latest post about fixing the nytimes zombie [1]. It could also be because some structure doesn't have the right CC macros; it's missing a field, for example. I don't have a good bug as an example of this, but maybe mccr8 does... [1] http://robert.ocallahan.org/2012/05/firefox-vs-new-york-times.html
I have just found something that may be related to this problem. Follow these steps: 1. Visit http://www.mozilla.org/en-US/ 2. Press CTRL+F to raise the search bar. 3. Type 'a' to the search field. 4. Click on 'Highlight all'. 5. Press CTRL+T to open a new tab. 6. Close the new tab or switch back to the previous tab. 7. Press CTRL+F to focus the search field. 8. Type 'b' to the search field (only 'b' should be in the search field right now). 9. Click on 'Highlight all'. All occurrences of 'a' and 'b' are highlighted, instead of just occurrences of 'b'. The 'Highlight all' feature works as expected (meaning occurrences of 'a' cease to be highlighted) if steps 5, 6 and 9 are skipped. Should I fill this as a new bug?
Yes, please file a new bug. This bug is just about zombie compartments.
(In reply to Olli Pettay [:smaug] from comment #7) > Yes, please file a new bug. This bug is just about zombie compartments. Filed as bug 760780.
Mano, could you look at this? Based on CVS blame you should know this code ;) If I read the code correctly _highlightDoc ends up creating ranges etc which are kept alive, but not released until something else is searched. https://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/findbar.xml#919
Component: General → Find Toolbar
Product: Firefox → Toolkit
QA Contact: general → fast.find
Attached patch patch? (deleted) — Splinter Review
Mano, is this all wrong? I don't see where else _searchRange, _startPt and _endPt are used, so why do they need to be stored in 'this'?
Attachment #629467 - Flags: review?(mano)
(And yes, the patch does fix the leak :) )
Comment on attachment 629467 [details] [diff] [review] patch? The code using this members used to live in few functions, but it doesn't anymore, so this change is fine. Thanks.
Attachment #629467 - Flags: review?(mano) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee: nobody → bugs
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: