nsPref.cpp memory management sucks

RESOLVED FIXED in Future

Status

()

Core
Preferences: Backend
P3
normal
RESOLVED FIXED
18 years ago
17 years ago

People

(Reporter: dougt, Assigned: Brian Nesse (gone))

Tracking

({embed, helpwanted, mlk})

Trunk
Future
All
Windows NT
embed, helpwanted, mlk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3-])

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
In many places this occurs:

if ()
{
 nsIFoo* specialChild2;
 SomeGetter(&specialChild2)
 if (anotherCondition)
    continue;  // just lost nsIFoo*
}
(Reporter)

Comment 1

18 years ago
Created attachment 11882 [details] [diff] [review]
Fixes 32 byte leak - use nsCOMPtrs!
(Reporter)

Updated

18 years ago
Keywords: embed, footprint, nsbeta2

Comment 2

18 years ago
+
Keywords: footprint → mlk, patch
Whiteboard: nsbeta2+

Comment 3

18 years ago
cc'ing alecf, who sometimes fixes nsPref bugs.

Comment 4

18 years ago
yikes. Yes, I'll take this while I fix the 4197 other pref leaks :(
Assignee: dveditz → alecf
over to jrgm, who also looks into leaky issues. ;)
QA Contact: sairuh → jrgm

Comment 6

18 years ago
(I'll checkin for beta3, btw)
(Reporter)

Comment 7

18 years ago
There are other problems with this file that I saw.  It is in need of some 
memory handling lovin.  It looks like we thought we were programming in java or 
something.

Comment 8

18 years ago
I think just nobody understood XPCOM at the time that it was written.
There are lots of small, specific leaks that I know about, and one wierd one
with the PL_hashtables that I need to investigate.. 

I just realized this was approved nsbeta2. I'd like to contest this because I
don't think it's worth the risk to fix these small, almost inconsequential leaks
within days of the beta. Jud?
Keywords: nsbeta3

Comment 9

18 years ago
point taken. -> beta3
Keywords: nsbeta2
Whiteboard: nsbeta2+ → nsbeta3+

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → M18

Comment 10

18 years ago
ok, patch is in. thanks doug.
leaving bug open so I can keep hacking at this stuff

Updated

18 years ago
Whiteboard: nsbeta3+ → [nsbeta3+]

Comment 11

17 years ago
Alec, no activity on this bug in over a month.  Do you know anything today that
prevents us from cutting this?

If you have specific fixes in mind, please describe them and their impact and
we'll reconsider.
Whiteboard: [nsbeta3+] → [b3 need info][cut 9/6]

Comment 12

17 years ago
yeah, I think this is ok for now, I have a cleanup or two that will go in to fix 
42102, so let's minus this for now

removing status for nsbeta3- consideration. 

Keeping the bug open because there's still work to do, even if it's post-rtm. 
Purify shows me leaked prefs with some of my builds, though not all...and I 
haven't figured out what is triggering it.
Whiteboard: [b3 need info][cut 9/6]

Comment 13

17 years ago
nsbeta3-
Whiteboard: [nsbeta3-]
Target Milestone: M18 → Future
Keywords: helpwanted

Updated

17 years ago
Target Milestone: Future → mozilla0.9

Comment 14

17 years ago
removing nsbeta3 keywords
Keywords: nsbeta3

Comment 15

17 years ago
How about getting this patch in, if it is correct? Let's create some activity
around this bug.
(Assignee)

Comment 16

17 years ago
As per  Alec Flett 2000-07-31 13:57 the patch is checked in. Bug was kept open
as a reminder.

Comment 17

17 years ago
Marking Future to get off the radar since this bug seems to be a reminder bug 
now.
Target Milestone: mozilla0.9 → Future
(Assignee)

Comment 18

17 years ago
Bulk assigning bugs covered by the rewrite of nsPrefs to myself.
Assignee: alecf → bnesse
Status: ASSIGNED → NEW
(Assignee)

Comment 19

17 years ago
Bulk updating some fields and accepting
Status: NEW → ASSIGNED
Keywords: nsbeta1
Hardware: PC → All
Target Milestone: Future → mozilla0.9
(Assignee)

Updated

17 years ago
Target Milestone: mozilla0.9 → Future
(Assignee)

Comment 20

17 years ago
As this patch was checked in, and nspref.cpp is now just a temporary wrapper for
calls into nsPrefService and nsPrefBranch, I'm going to close this bug to get it
out of my bug list.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.