PrefEnumCallback leaks strings

VERIFIED FIXED in mozilla0.9.2

Status

()

P2
major
VERIFIED FIXED
18 years ago
17 years ago

People

(Reporter: dbaron, Assigned: ftang)

Tracking

({intl, memory-leak, top-memory-leak})

Trunk
mozilla0.9.2
All
Linux
intl, memory-leak, top-memory-leak
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PDT+]wait for tree open to check in .)

Attachments

(2 attachments)

The changes made by bstell in revision 1.140 of nsFontMetricsGTK.cpp for bug
67732 and bug 69139 cause PrefEnumCallback to leak the string |value| whenever
|name.Equals(value)|.  This is causing rather significant leaks.
Oops, meant to assign to bstell.
Assignee: nhotta → bstell
Keywords: mlk, topmlk

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.2

Updated

18 years ago
Priority: -- → P1
(Assignee)

Comment 2

18 years ago
I believe dbaron mean that the value is not free from block start from 
3304   if ((value) && (!name.Equals(value))) {

we should add a 
if(value) {
 nsMemory::Free(value);
 value = nsnull;
}

right before return.

  
(Assignee)

Comment 3

18 years ago
bstell is overload. mark it moz0.9.2 P2 and move it to ftang
Priority: P1 → P2
(Assignee)

Comment 4

18 years ago
bstell is overload. mark it moz0.9.2 P2 and move it to ftang
Assignee: bstell → ftang
Status: ASSIGNED → NEW

Updated

18 years ago
Keywords: intl
(Assignee)

Comment 5

18 years ago
pdt+ base on 6/11 pdt meeting.
(Assignee)

Updated

18 years ago
Whiteboard: [PDT+]
(Assignee)

Comment 6

18 years ago
Created attachment 38002 [details] [diff] [review]
patch to fix leak
(Assignee)

Comment 7

18 years ago
                                                                              
dbaron@fas.harvard.edu (David Baron)
can you review the patch?
Status: NEW → ASSIGNED
That patch compares |name| to |value| after setting |value| to |nsnull|.  It 
would be easier to just use nsXPIDLCString (and better proofed against future 
changes).

Comment 9

18 years ago
Created attachment 38130 [details] [diff] [review]
patch using xpidl string
r=dbaron, although you should probably also remove the explicit nulling of
|value| that was right after the two calls to |nsMemory::Free| that you removed.

Comment 11

18 years ago
sr=kin@netscape.com with the change that dbaron suggested.
(Assignee)

Updated

18 years ago
Whiteboard: [PDT+] → [PDT+]r=dbaron sr=kin need a=

Comment 12

18 years ago
*** Bug 83748 has been marked as a duplicate of this bug. ***

Comment 13

18 years ago
ftang, what's up? can we get this checked in?
(Assignee)

Comment 14

18 years ago
I am waiting for a=
a=dbaron for trunk checkin (on behalf of drivers), assuming you make the change
requested above.

In general, though, if you want a=, please email drivers@mozilla.org asking for it.
(Assignee)

Updated

18 years ago
Whiteboard: [PDT+]r=dbaron sr=kin need a= → [PDT+]wait for tree open to check in .
(Assignee)

Comment 16

18 years ago
fixed and check in 
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 17

18 years ago
Changing QA contact to ftang@netscape.com for now.  Frank, can you verify this
within development or provide IQA with a test case?  Thanks.
QA Contact: andreasb → ftang
(Assignee)

Comment 18

17 years ago
verify
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.