53569 - leaks up from 1k to 808k

Status: RESOLVED FIXED

(SeaMonkey :: General, defect, P1)

Comment 1

[Jon Granrose]

new memory leaks are considered tree blockers.  adding smoketest keyword until
someone claims this bustage.

Comment 2

[Joe Francis]

mstoltz checkin to nsXMLDocument.cpp is one leak: unneeded addref at line 276.
erik checkin to nsFontMetricsGTK.cpp is another: missing release for "converter" 
in routine "CheckMap()".  Also an old leak like that one a few lines down.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is it known whether either of these is the problem on tinderbox?  Has anyone run
the refcount balancer on the leaked nsXULDocument?

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Attachment: Fix for XMLDocument leak

Comment 5

[Brendan Eich [:brendan]]

r=[e-mail] on that leak fix

One question, not priority because it has nothing to do with the leak: can 
GetCodebasePrincipal return a null principal pointer through the out parameter, 
but return NS_OK to rv?  If so, is that the right return code to propagate in 
the case where there is no principal?

/be

Comment 6

[anthonyd]

adding ftang since he r= erik's fix with the memory leak.

anthony

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

A tinderbox build that *did* have the nsXULDocument fix didn't show any
improvement in leak stats.  Has someone run the refcount balancer on the leaked
nsXULDocument?  I don't have a current tree, or a machine on which I can build
and display locally.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

It was nsXMLDocument::Load, not nsXULDocument, and I don't think that's code that 
gets run during the normal course of things, so it probably wasn't affecting the 
leak stats at all. Nonetheless, it's fixed.

Comment 9

[Erik van der Poel]

I checked in a fix for the new leak, and there is a bug for the old leak
(bug 53585).

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Looks like evaughan's changes fixed this...

Comment 11

[Mike Pinkerton (not reading bugmail)]

this was evaughan, and is now fixed. is this still valid? removing from
smoketest list.

Comment 12

[Warren Harris]

=> evaughan

Comment 13

[Peter Trudelle]

resolving as fixed.

Comment 14

[Doron Rosenberg (IBM)]

anyone on linux who can verify this?

Comment 15

[Daniel Bratell]

There is still 7 KB left of the leak. (It was 1 KB before the trouble and 
Tinderbox shows 8 KB now). Before verifieing this bug, please check that those 
are covered by some other bug so that they are not lost.