458844 - 'bugs' and 'mathml' reftests leak MathML objects

Status: VERIFIED FIXED

(Core :: MathML, defect)

Description

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20081004 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)

{
   6 nsDisplayBackground
3803 nsMathMLContainerFrame
  61 nsMathMLmtableOuterFrame
  64 nsMathMLmtdInnerFrame
}

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: Test and Leak log

Comment 2

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080929201016 Minefield/3.1b1pre] (home, optim default) (W2Ksp4)

FF has this leak too:
this build reports "6 / 6069 / 85 / 72" leaked objects.

Comment 3

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081009 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)

Same leak, with additional
{
   1 nsBaseDragService
  24 nsNativeDragTarget
}

Comment 4

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081122 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/5d37678a2482
+http://hg.mozilla.org/comm-central/rev/892e9f783d9e)

With only
{
# mathml/
include mathml/reftest.list
}
I (now) get
{
     |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->|
                                              Per-Inst   Leaked    Total      Rem      Mean       StdDev     Total      Rem      Mean       StdDev
   0 TOTAL                                          37    96020   201016      815 (  692,98 +/-   950,62)   248305      815 (  581,21 +/-  1099,66)
 345 nsMathMLContainerFrame                        116    86420      745      745 (  373,00 +/-   215,21)      745      745 (  373,00 +/-   215,21)
 347 nsMathMLmtableOuterFrame                      132     5280       40       40 (   20,50 +/-    11,69)       40       40 (   20,50 +/-    11,69)
 348 nsMathMLmtdInnerFrame                         144     4320       30       30 (   15,50 +/-     8,80)       30       30 (   15,50 +/-     8,80)
}

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #3)
> Same leak, with additional
> {
>    1 nsBaseDragService
>   24 nsNativeDragTarget
> }

Moved to bug 466376.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)

And, with only
{
# bugs/
include bugs/reftest.list
}
I get
{
     |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->|
                                              Per-Inst   Leaked    Total      Rem      Mean       StdDev     Total      Rem      Mean       StdDev
   0 TOTAL                                          42   362492 14977012     3119 (  771,31 +/-  1168,19) 19796382     3113 (  614,81 +/-  1270,60)
 301 nsDisplayBackground                            16       96    13169        6 (   18,33 +/-    38,69)        0        0 (    0,00 +/-     0,00)
 517 nsMathMLContainerFrame                        116   354728     3058     3058 ( 1529,50 +/-   882,91)     3058     3058 ( 1529,50 +/-   882,91)
 519 nsMathMLmtableOuterFrame                      132     2772       21       21 (   11,00 +/-     6,20)       21       21 (   11,00 +/-     6,20)
 520 nsMathMLmtdInnerFrame                         144     4896       34       34 (   17,50 +/-     9,96)       34       34 (   17,50 +/-     9,96)
}

(In reply to comment #0)
> {
>    6 nsDisplayBackground
> 3803 nsMathMLContainerFrame
>   61 nsMathMLmtableOuterFrame
>   64 nsMathMLmtdInnerFrame
> }

Actually, the leak initially reported is the sum of 'bugs' and 'mathml' lists (leaks).

Comment 7

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081209
Minefield/3.2a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/85507cfcdda8)

I ran each 'mathml' test separately:
mi-mathvariant-1.xhtml     :  24 nsMathMLContainerFrame.
mi-mathvariant-2.xhtml     :  16 nsMathMLContainerFrame.
table-width-1.xhtml        : 735 nsMathMLContainerFrame, 58 nsMathMLmtableOuterFrame, 30 nsMathMLmtdInnerFrame.
overbar-width-1.xhtml      : 101 nsMathMLContainerFrame.
underbar-width-1.xhtml     : 101 nsMathMLContainerFrame.
non-spacing-accent-1.xhtml :  62 nsMathMLContainerFrame.
stretchy-underbar-1.xhtml  :  66 nsMathMLContainerFrame.

Obvious leak is nsMathMLContainerFrame ... which is bug 391976 !
(All) The other leaks may only be side-effects.

Comment 8

[Boris Zbarsky [:bzbarsky]]

The mathml frame isn't actually leaked, possibly.  The refcounting is just mis-logged for them.  I doubt it has side effects.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Should be fixed now, there is no longer addref/release for MathML frames.

Comment 10

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090114 Minefield/3.2a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/e0ed5852481a)

V.Fixed

(In reply to comment #0)
>    6 nsDisplayBackground

I filed bug 473548 about this (remaining/other) part.

Comment 11

[Karl Tomlinson (:karlt)]

Removing wanted1.9.1, as this is not a real leak but a logging issue (see bug 391976).  This is fixed by bug 396185, so if the fix is really wanted on 1.9.1, it should be considered there.

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)
> This is fixed by bug 396185, so if the fix is really wanted on 1.9.1,
> it should be considered there.

Asking there ... This bug(s) will remain on 1.9.1 branch.

Comment 13

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

If you wanted to fix this on 1.9.1, you could just make the MathML frame classes stop using NS_IMPL_ADDREF_INHERITED and NS_IMPL_RELEASE_INHERITED (i.e., make a separate version that doesn't log).

Comment 14

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)
> This is fixed by bug 396185, so if the fix is really wanted on 1.9.1,

Reasking here, as that fix won't land (as is) on branch.


(In reply to comment #13)
> If you wanted to fix this on 1.9.1

Bug 469518 comment 25:
{
*FF 3.1: bug 458844, expected as not fixed there.
}
and the tinderboxes report this bug on the crashtest suite too.

I'd love to see this leak report fixed, as it's the last reported "leak".

David, could you do it ? (That's beyond what I can do.)

Comment 15

[Serge Gautherie (:sgautherie)]

Ftr, I hacked a workaround leak threshold in bug 469518 but I would like to get rid of it.

Comment 16

[Serge Gautherie (:sgautherie)]

verified1.9.1 per bug 391976 comment 9.


(In reply to comment #15)
> Ftr, I hacked a workaround leak threshold in bug 469518 but I would like to get
> rid of it.

Now I can ;-)