Last Comment Bug 605732 - test_taskbar_jumplistitems.js leaks items
: test_taskbar_jumplistitems.js leaks items
Status: RESOLVED FIXED
[xpcshell-leak][qa-]
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla9
Assigned To: Brian R. Bondy [:bbondy]
:
: Jim Mathies [:jimm]
Mentors:
Depends on:
Blocks: 469523
  Show dependency treegraph
 
Reported: 2010-10-19 23:23 PDT by Josh Matthews [:jdm] (on vacation until Dec 5)
Modified: 2011-09-09 15:20 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
test_taskbar_jumplistitems.js leaks items (702 bytes, patch)
2011-08-24 19:28 PDT, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
test_taskbar_jumplistitems.js leaks items v2 (701 bytes, patch)
2011-08-24 21:03 PDT, Brian R. Bondy [:bbondy]
jmathies: review+
Details | Diff | Splinter Review
Rebased to mozilla-central tip so it doesn't require my jump list icon task (661 bytes, patch)
2011-08-25 07:57 PDT, Brian R. Bondy [:bbondy]
netzen: review+
jst: approval‑mozilla‑aurora+
jst: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Josh Matthews [:jdm] (on vacation until Dec 5) 2010-10-19 23:23:55 PDT
== 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] 2010-10-20 09:47:16 PDT
I wrote this so I suppose I should fix it. How series is this?
Comment 2 Josh Matthews [:jdm] (on vacation until Dec 5) 2010-10-20 10:00:05 PDT
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] 2011-08-24 18:25:53 PDT
I noticed these leaks when running this test so I'll take this one.
Comment 4 Brian R. Bondy [:bbondy] 2011-08-24 19:28:49 PDT
Created attachment 555620 [details] [diff] [review]
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] 2011-08-24 19:29:55 PDT
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] 2011-08-24 21:03:56 PDT
Created attachment 555630 [details] [diff] [review]
test_taskbar_jumplistitems.js leaks items v2

Accidentally had an extra semicolon.
Comment 7 Brian R. Bondy [:bbondy] 2011-08-25 07:11:33 PDT
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] 2011-08-25 07:39:05 PDT
Comment on attachment 555630 [details] [diff] [review]
test_taskbar_jumplistitems.js leaks items v2

oops.
Comment 9 Brian R. Bondy [:bbondy] 2011-08-25 07:45:41 PDT
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] 2011-08-25 07:57:21 PDT
(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] 2011-08-25 07:57:56 PDT
Created attachment 555733 [details] [diff] [review]
Rebased to mozilla-central tip so it doesn't require my jump list icon task
Comment 12 Brian R. Bondy [:bbondy] 2011-08-25 07:58:41 PDT
ok cool, thanks for the info!
Comment 13 Jim Mathies [:jimm] 2011-08-25 07:59:05 PDT
Out of curiosity, worst case how big is the leak?
Comment 14 Brian R. Bondy [:bbondy] 2011-08-25 08:01:55 PDT
15KiB every 2 minutes that the browser is left open.  Or 10MiB/day.
Comment 15 Brian R. Bondy [:bbondy] 2011-08-25 08:03:03 PDT
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] 2011-08-25 08:05:43 PDT
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] 2011-08-25 18:25:10 PDT
http://hg.mozilla.org/mozilla-central/rev/8177de770dd2
Comment 18 Johnny Stenback (:jst, jst@mozilla.com) 2011-08-29 14:32:50 PDT
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 20 :Ehsan Akhgari 2011-09-07 12:37:24 PDT
I filed bug 685266 to get a static analysis which checks for this case.
Comment 21 Brian R. Bondy [:bbondy] 2011-09-07 12:39:35 PDT
I tried searching briefly for similar errors but didn't find any at the time.  Great idea Ehsan, thanks.

Note You need to log in before you can comment on or make changes to this bug.