605732 - test_taskbar_jumplistitems.js leaks items

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Josh Matthews [:jdm]]

== BloatView: ALL (cumulative) LEAK STATISTICS, default process 1928

     |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->|
                                              Per-Inst   Leaked    Total      Rem      Mean       StdDev     Total      Rem      Mean       StdDev
   0 TOTAL                                          21      244     5574       10 (  310.86 +/-   408.18)    15681       10 ( 1666.64 +/-  1933.76)
  52 nsCryptoHash                                   28       28        1        1 (    1.00 +/-     0.00)        3        1 (    1.40 +/-     0.55)
 114 nsSimpleURI                                    56      168        5        3 (    3.14 +/-     1.35)       63        3 (   10.99 +/-     5.24)
 119 nsStringBuffer                                  8       48     1705        6 (  791.34 +/-   421.27)     8511        6 ( 3038.74 +/-  1664.35)

Comment 1

[Jim Mathies [:jimm]]

I wrote this so I suppose I should fix it. How series is this?

Comment 2

[Josh Matthews [:jdm]]

I'm one of numerous xpcshell tests leaking, but leaks don't turn the tree orange at the moment.

Comment 3

[Brian R. Bondy [:bbondy]]

I noticed these leaks when running this test so I'll take this one.

Comment 4

[Brian R. Bondy [:bbondy]]

Attachment: test_taskbar_jumplistitems.js leaks items

Virtual destructor was missing on the base class causing the derived class' destructor to not be hit.

Comment 5

[Brian R. Bondy [:bbondy]]

I think this one should probably be promoted above Nightly by the way because I think every 120 seconds it will leak more and more memory.  If you leave the browser open this can contribute to it getting slower.

Comment 6

[Brian R. Bondy [:bbondy]]

Attachment: test_taskbar_jumplistitems.js leaks items v2

Accidentally had an extra semicolon.

Comment 7

[Brian R. Bondy [:bbondy]]

This leak is probably not seen by other tests because it takes 120 seconds to start a new jumplist build and I'm not sure if any test runs that long.

Comment 8

[Jim Mathies [:jimm]]

Comment on attachment 555630 [details] [diff] [review]
test_taskbar_jumplistitems.js leaks items v2

oops.

Comment 9

[Brian R. Bondy [:bbondy]]

I will push this to inbound, but should this be also pushed to aurora or beta channels?  If so what is the process for me to do that or request that?

Comment 10

[Jim Mathies [:jimm]]

(In reply to Brian R. Bondy [:bbondy] from comment #9)
> I will push this to inbound, but should this be also pushed to aurora or
> beta channels?  If so what is the process for me to do that or request that?

You can request branch landing via flags on the patch once it's landed. Drivers will then decide if it's worth the risk or if it should roll out normally.

Comment 11

[Brian R. Bondy [:bbondy]]

Attachment: Rebased to mozilla-central tip so it doesn't require my jump list icon task

Comment 12

[Brian R. Bondy [:bbondy]]

ok cool, thanks for the info!

Comment 13

[Jim Mathies [:jimm]]

Out of curiosity, worst case how big is the leak?

Comment 14

[Brian R. Bondy [:bbondy]]

15KiB every 2 minutes that the browser is left open.  Or 10MiB/day.

Comment 15

[Brian R. Bondy [:bbondy]]

Sorry that's if the leak is the same as what the xpcshell leaks.  I'm guessing it is +/- similar though.

Comment 16

[Brian R. Bondy [:bbondy]]

I'm sure it's fine but I'm running though try since I'm paranoid:

http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=45a9fd7932c4

Comment 17

[Ed Morley [:emorley]]

http://hg.mozilla.org/mozilla-central/rev/8177de770dd2

Comment 18

[Johnny Stenback (:jst)]

Comment on attachment 555733 [details] [diff] [review]
Rebased to mozilla-central tip so it doesn't require my jump list icon task

Approved to land for aurora and beta per todays driver meeting.

Comment 19

[LegNeato]

http://hg.mozilla.org/releases/mozilla-aurora/rev/abb9bc5f7993
http://hg.mozilla.org/releases/mozilla-beta/rev/e8c0eb11e0c1

Comment 20

[(no longer active)]

I filed bug 685266 to get a static analysis which checks for this case.

Comment 21

[Brian R. Bondy [:bbondy]]

I tried searching briefly for similar errors but didn't find any at the time.  Great idea Ehsan, thanks.