15820 - [DOGFOOD]MLK: every ImageManagerImpl leaks

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P3)

Description

[Warren Harris]

Bloaty: Refcounting and Memory Bloat
Statistics

|<-------Name------>|<--------------References-------------->|<----------------
Objects---------------->|<------Size----->|
                               Rem    Total      Mean       StdDev       Rem
Total      Mean       StdDev Per-Class      Rem
 257 ImageManagerImpl           93       93 (   47.00 +/-    46.33)        1
1 (    1.00 +/-     0.00)       12       12

Comment 1

[Jim Roskind]

How much, how often?

Comment 2

[leger]

We need more info to assist in PDT+ vs. PDT- decision.  How bad is this leak?
Thanks!

Comment 3

[pnunn]

Running apprunner/mozilla. First site is mozilla.org, sample#2, sample#0:
ImageManagerImpl: 12 bytes/instance, and total leak of 12 bytes.

Comment 4

[pnunn]

*** Bug 14439 has been marked as a duplicate of this bug. ***

Comment 5

[leger]

This leak not big enough for dogfood.  Marking [PDT-]

Comment 6

[pnunn]

Neeti has a fix for this.
-pn

Comment 7

[Warren Harris]

If it's the fix I submitted to you, then I approve the checkin.

Comment 8

[neeti]

Will need to make ImageManagerImpl a service. Moving tfv to M12.

Comment 9

[neeti]

This was fixed by making ImageManagerImpl a service on windows, mac and gtk

Comment 10

[Eli Goldberg]

Warren,

Does neeti's fix address this memory leak to your full satisfaction, or shall I
just rubber-stamp this as verified?

Thanks!

Comment 11

[Warren Harris]

You can verify by looking at the tinderbox logs on coffee. You should see that
not every ImageManagerImpl leaks. Let me know if you can't verify this.

Comment 12

[Eli Goldberg]

Putting aside for a moment the fact that I don't know what I'm doing ;)...

...from the current BloatView on Seamonkey Tinderbox for ImageManagerImpl, using
a depend build from 11.29 1:06 PM:

 	Per-Inst   Leaked    Total      Rem      Mean       StdDev     Total
         12        0        1        0 (    0.50 +/-     0.87)       18

Rem      Mean       StdDev
 0 (    3.33 +/-     2.66)

-----

So, it's still leaking 12 bytes when it leaks, but the mean is down by over an
order of magnitude. (I'm not sure what it's the mean _of_, however. ;)

Warren says?

Comment 13

[Simon Fraser [no longer active]]

I'm still seeing native image data leak when laoding a page with images in
appruner, then closing teh window (you can see this in ZoneRanger). It may not be
this specific bug, but it's very serious.

Comment 14

[neeti]

Simon,

Is the ImageURLImpl class leaking leaking? If not, can you give me the name of
the class which is leaking?

Comment 15

[Warren Harris]

Eli: The Total column is the number allocated, Rem is the number remaining which
is 0, so this isn't leaking any more. Thanks for checking. Now you'll know how
to verify the next one.

Simon: You should report a new bug against Pam about mac native image data
leaking.

Comment 16

[Eli Goldberg]

Rubber-stamping as verified.

(Warren's effort is appreciated; browser QA has been suggested not to verify
memory leak bugs.)