Closed Bug 460532 Opened 17 years ago Closed 17 years ago

Firefox leaks SelectionDetails after running for a while

Categories

(Core :: General, defect, P2)

x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: cbook, Assigned: uriber)

References

Details

(Keywords: fixed1.9.1, memory-leak)

Attachments

(2 files)

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081017 Firefox/3.1b2pre Steps to reproduce: -> 3 Tabs open with various sites + running firefox for a while and and was filing a bug :) --> Leak == BloatView: ALL (cumulative) LEAK STATISTICS |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->| Per-Inst Leaked Total Rem Mean StdDev Total Rem Mean StdDev 0 TOTAL 23 32 18138505 2 ( 4630.88 +/- 3494.33) 32592776 0 ( 4443.49 +/- 6241.90) 74 SelectionDetails 16 32 4646 2 ( 2.13 +/- 0.98) 0 0 ( 0.00 +/- 0.00) nsTraceRefcntImpl::DumpStatistics: 939 entries nsStringStats => mAllocCount: 184172 => mReallocCount: 22570 => mFreeCount: 184172 => mShareCount: 172248 => mAdoptCount: 18508 => mAdoptFreeCount: 18508
Attached patch patchSplinter Review
Funny coincidence - I just stumbled upon this code a couple of days ago and noticed it's leaky. This patch should fix the leak, and also gets rid of an unnecessary "if".
Assignee: nobody → uriber
Status: NEW → ASSIGNED
Attachment #343736 - Flags: review?(roc)
Not sure if this is a regression. If it is it might be a blocker.
Flags: blocking1.9.1?
Comment on attachment 343736 [details] [diff] [review] patch awesome, thanks! So to reproduce this, you just have to have a multiple selection and click in the first part of the selection, right? Can we write a mochitest for that that leaks without this fix? I guess what we really want is for SelectionDetails to not be such an error-prone mess...
Attachment #343736 - Flags: superreview+
Attachment #343736 - Flags: review?(roc)
Attachment #343736 - Flags: review+
(In reply to comment #3) > So to reproduce this, you just have to have a multiple selection and click in > the first part of the selection, right? Not exactly - you need more than one selection range (either of the same type or of different types) to overlap the clicked point (in practice, this probably occurs when you click in a selection that is also marked as misspelled or highlighted by "Highlight all"). I'll try to make a mochitest with two overlapping normal selection ranges.
Attached patch mochitestSplinter Review
This leaks one SelectionDetails object without the fix.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Uri left a message on the tinderbox waterfall at 16:32 that he'd pushed a fix for the test bustage (on linux, mochitest is orange with focus errors after starting test_word_movement.html; on win32, redness from hanging test_character_movement.html). That doesn't appear in the pushlog. #developers suggested disabling the new test for this bug, to try to isolate if it leaves mochitest in a bad state for subsequent tests, or if there is a problem with the code change. That's http://hg.mozilla.org/mozilla-central/rev/fba837242af0
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #8) > Uri left a message on the tinderbox waterfall at 16:32 that he'd pushed a fix > for the test bustage (on linux, mochitest is orange with focus errors after > starting test_word_movement.html; on win32, redness from hanging > test_character_movement.html). That doesn't appear in the pushlog. Sorry. My push failed and I didn't notice. I now pushed the fix to the test and re-enabled it. I'll follow the tree.
Seems to be working now.
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: