46379 - nsPref.cpp memory management sucks

Status: RESOLVED FIXED

(Core :: Preferences: Backend, defect, P3)

Description

[Doug Turner (:dougt)]

In many places this occurs:

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

Comment 1

[Doug Turner (:dougt)]

Attachment: Fixes 32 byte leak - use nsCOMPtrs!

Comment 2

[Judson Valeski]

+

Comment 3

[Chris Waterson]

cc'ing alecf, who sometimes fixes nsPref bugs.

Comment 4

[Alec Flett]

yikes. Yes, I'll take this while I fix the 4197 other pref leaks :(

Comment 5

[sairuh (rarely reading bugmail)]

over to jrgm, who also looks into leaky issues. ;)

Comment 6

[Alec Flett]

(I'll checkin for beta3, btw)

Comment 7

[Doug Turner (:dougt)]

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

[Alec Flett]

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?

Comment 9

[Judson Valeski]

point taken. -> beta3

Comment 10

[Alec Flett]

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

Comment 11

[selmer (gone)]

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.

Comment 12

[Alec Flett]

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.

Comment 13

[selmer (gone)]

nsbeta3-

Comment 14

[Alec Flett]

removing nsbeta3 keywords

Comment 15

[Håkan Waara]

How about getting this patch in, if it is correct? Let's create some activity
around this bug.

Comment 16

[Brian Nesse (gone)]

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

Comment 17

[Paul Chen]

Marking Future to get off the radar since this bug seems to be a reminder bug 
now.

Comment 18

[Brian Nesse (gone)]

Bulk assigning bugs covered by the rewrite of nsPrefs to myself.

Comment 19

[Brian Nesse (gone)]

Bulk updating some fields and accepting

Comment 20

[Brian Nesse (gone)]

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.