Last Comment Bug 735574 - Extremely frequent leak of 1 BackstagePass, 2 Mutex, 9 ProxyListener coming from New Tab Page (mochitest-other)
: Extremely frequent leak of 1 BackstagePass, 2 Mutex, 9 ProxyListener coming f...
Status: RESOLVED FIXED
[fixed by bug 738774] [MemShrink:P1]
: intermittent-failure, mlk, regression
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 14
Assigned To: Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
:
Mentors:
Depends on: 738774
Blocks: 438871 729878
  Show dependency treegraph
 
Reported: 2012-03-13 23:26 PDT by Phil Ringnalda (:philor)
Modified: 2012-11-25 19:31 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
affected


Attachments
patch v1 [Checked in: Comment 47] (5.63 KB, patch)
2012-03-15 09:41 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
dietrich: review+
Details | Diff | Splinter Review

Description Phil Ringnalda (:philor) 2012-03-13 23:26:22 PDT
https://tbpl.mozilla.org/?tree=Fx-Team&rev=c7e4db80d280 is when bug 729111 landed. Note both the unstarred leak (until I star it as this) in WinXP debug and the starred failure on 10.5 debug mochitest-other, which was both a test failure and this leak, but only starred as the test failure.

https://tbpl.mozilla.org/?rev=ee4e0c98cb02 was when it was merged to mozilla-central, with just one leak where I retriggered 10.5 debug while chasing this backward. From there, though, it explodes.

https://tbpl.mozilla.org/php/getParsedLog.php?id=10026260&tree=Fx-Team&full=1
Rev3 WINNT 5.1 fx-team debug test mochitest-other on 2012-03-13 00:46:30 PDT for push c7e4db80d280

TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 36892 bytes during test execution
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of BackstagePass with size 24 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 2 instances of Mutex with size 12 bytes each (24 bytes total)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 9 instances of ProxyListener with size 16 bytes each (144 bytes total)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of ReentrantMonitor with size 16 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of SharedScriptableHelperForJSIID with size 12 bytes
TEST-INFO | automationutils.processLeakLog() | leaked 2 instances of XPCNativeScriptableInfo with size 8 bytes each (16 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 9 instances of XPCNativeScriptableShared with size 232 bytes each (2088 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 253 instances of XPCWrappedNative with size 56 bytes each (14168 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 24 instances of XPCWrappedNativeProto with size 32 bytes each (768 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 9 instances of imgCacheValidator with size 60 bytes each (540 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 18 instances of imgRequest with size 176 bytes each (3168 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 9 instances of imgRequestProxy with size 68 bytes each (612 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 1 instance of nsDocShell::InterfaceRequestorProxy with size 16 bytes
TEST-INFO | automationutils.processLeakLog() | leaked 45 instances of nsJSCID with size 52 bytes each (2340 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 177 instances of nsJSIID with size 24 bytes each (4248 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 18 instances of nsPrincipal with size 56 bytes each (1008 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 9 instances of nsProgressNotificationProxy with size 28 bytes each (252 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 18 instances of nsProperties with size 8 bytes each (144 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 19 instances of nsSimpleURI with size 72 bytes each (1368 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 56 instances of nsStringBuffer with size 8 bytes each (448 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 1 instance of nsSystemPrincipal with size 16 bytes
TEST-INFO | automationutils.processLeakLog() | leaked 36 instances of nsTArray_base with size 4 bytes each (144 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 28 instances of nsVoidArray with size 4 bytes each (112 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 1 instance of nsWeakReference with size 16 bytes
TEST-INFO | automationutils.processLeakLog() | leaked 2 instances of nsXPCComponents with size 60 bytes each (120 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 2 instances of nsXPCComponents_Classes with size 20 bytes each (40 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 2 instances of nsXPCComponents_Interfaces with size 28 bytes each (56 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 2 instances of nsXPCComponents_Results with size 20 bytes each (40 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 2 instances of nsXPCComponents_Utils with size 24 bytes each (48 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 18 instances of nsXPCWrappedJS with size 64 bytes each (1152 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 2 instances of nsXPCWrappedJSClass with size 44 bytes each (88 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 172 instances of xptiInterfaceInfo with size 20 bytes each (3440 bytes total)
TEST-INFO | automationutils.processLeakLog() | leaked 1 instance of xptiInterfaceInfoManager with size 128 bytes
TEST-INFO | automationutils.processLeakLog() | leaked 1 instance of xptiWorkingSet with size 88 bytes
Comment 7 Charles Chan 2012-03-14 09:12:47 PDT
Since might be responsible for causing the bug, how should I tackle the cause of this issue?

(I also commented in bug 729111 https://bugzilla.mozilla.org/show_bug.cgi?id=729111#c20)
Comment 12 Charles Chan 2012-03-14 12:59:31 PDT
Changes for bug 729111 has been backed out of Mozilla-Inbound:
https://bugzilla.mozilla.org/show_bug.cgi?id=729111#c22
Comment 13 Charles Chan 2012-03-14 13:42:49 PDT
(Copied from https://bugzilla.mozilla.org/show_bug.cgi?id=729111#c24)

It looks like the build was still failing after the changes were backed out:
114c4148c6f3 Justin Lebar – Back out bug 729111 (rev c7e4db80d280) due to suspected randomorange (bug 735574).
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=114c4148c6f3

But the next build seems to pass:
d067c50e01dc Ehsan Akhgari – Backout changeset ea6be5f60c42 (bug 722946) for breaking Windows builds
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d067c50e01dc

Would that mean the problem is not caused by bug 729111?
Comment 16 Ryan VanderMeulen [:RyanVM] 2012-03-14 17:01:27 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10072404&tree=Mozilla-Inbound
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 62552 bytes during test execution
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of BackstagePass with size 48 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 2 instances of Mutex with size 24 bytes each (48 bytes total)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 5 instances of ProxyListener with size 32 bytes each (160 bytes total)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of ReentrantMonitor with size 32 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of SharedScriptableHelperForJSIID with size 24 bytes
Comment 17 Phil Ringnalda (:philor) 2012-03-14 17:08:56 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10069704&tree=Firefox

Apparently I put too much faith in the number of debug mochitest-others that had run on the push before Charles' - it's already showing this same leak on more retriggers. Sorry, we'll get you relanded.
Comment 21 Charles Chan 2012-03-14 19:08:39 PDT
Thanks Phil for the clarifications. :)
Comment 24 Phil Ringnalda (:philor) 2012-03-14 21:38:49 PDT
So, two pushes before that was https://tbpl.mozilla.org/?tree=Fx-Team&rev=bb271ef702c6 which says "no, no, can't say as I'd ever be willing to leak that way," and one push before it was https://tbpl.mozilla.org/?tree=Fx-Team&rev=b2a4560c0af0 which grudgingly admits that it would in fact leak exactly like that. Hello, bug 729878.
Comment 28 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-15 09:41:11 PDT
Created attachment 606263 [details] [diff] [review]
patch v1
[Checked in: Comment 47]

This seems to fix the leak for me. Couldn't reproduce it on try though it was really frequent on pushes for other patches.
Comment 32 Dietrich Ayala (:dietrich) 2012-03-15 12:24:19 PDT
Comment on attachment 606263 [details] [diff] [review]
patch v1
[Checked in: Comment 47]

Review of attachment 606263 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/modules/NewTabUtils.jsm
@@ +549,5 @@
>    /**
>     * Resets the links cache.
>     */
>    resetCache: function Links_resetCache() {
> +    this._links = null;

hm, iirc you need to empty *and* null out the array. or that used to be the case. but since you're not seeing leaks anymore, consider this a note in case they come back ;)

@@ +583,5 @@
>   * Singleton that provides the public API of this JSM.
>   */
>  let NewTabUtils = {
>    /**
> +   * Restores all sites that haven been removed from the grid.

nit: typo

@@ +591,5 @@
>      Links.resetCache();
>      PinnedLinks.resetCache();
>      BlockedLinks.resetCache();
> +
> +    Links.populateCache(function () { AllPages.update() }, true);

hm, either break into lines or can you do Links.populateCache(AllPages.update, true) ?
Comment 34 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-15 15:13:12 PDT
(In reply to Dietrich Ayala (:dietrich) from comment #32)
> >    resetCache: function Links_resetCache() {
> > +    this._links = null;
> 
> hm, iirc you need to empty *and* null out the array. or that used to be the
> case. but since you're not seeing leaks anymore, consider this a note in
> case they come back ;)

Ok, I'll keep that in mind.

> > +   * Restores all sites that haven been removed from the grid.
> 
> nit: typo

Fixed.

> > +
> > +    Links.populateCache(function () { AllPages.update() }, true);
> 
> hm, either break into lines or can you do
> Links.populateCache(AllPages.update, true) ?

I'd have to .bind(this) so I just broke into lines.
Comment 35 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-15 15:33:20 PDT
https://hg.mozilla.org/integration/fx-team/rev/13872068ea17
Comment 40 Phil Ringnalda (:philor) 2012-03-15 17:29:59 PDT
Doesn't seem to have gotten it, since https://tbpl.mozilla.org/php/getParsedLog.php?id=10107616&tree=Fx-Team is in the push after yours.
Comment 45 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-16 00:41:49 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10116518&tree=Firefox
Comment 46 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-16 00:52:14 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10117901&tree=Fx-Team
Comment 47 Marco Bonardo [::mak] 2012-03-16 02:56:55 PDT
https://hg.mozilla.org/mozilla-central/rev/13872068ea17
Comment 108 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-19 05:04:33 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10176294&tree=Fx-Team
Comment 109 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-19 06:10:49 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10177345&tree=Fx-Team
Comment 110 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-19 06:49:34 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10177806&tree=Fx-Team
Comment 111 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-19 09:02:46 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10180489&tree=Fx-Team
Comment 132 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-20 04:21:41 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10208406&tree=Fx-Team
Comment 162 Andrew McCreight [:mccr8] 2012-03-21 16:30:05 PDT
Nominating for tracking because this is an "extremely frequent leak" in a new feature.  As far as I can tell, it hasn't been established whether this is the fault of the tests or the feature.
Comment 167 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-21 18:33:17 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10255986&tree=Mozilla-Aurora
Comment 168 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-21 18:34:36 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10252241&tree=Mozilla-Aurora&full=1
Comment 182 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-22 10:31:46 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10261380&tree=Fx-Team
Comment 183 Andrew McCreight [:mccr8] 2012-03-22 10:36:01 PDT
If 13 is still affected, we should still track it for that 13.
Comment 187 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-22 11:48:18 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10285905&tree=Firefox
Comment 189 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-22 12:57:28 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10288293&tree=Firefox
Comment 192 Daniel Holbert [:dholbert] 2012-03-22 14:33:21 PDT
 --> Updating summary to mention "new tab page" instead of just something vague "from fx-team", now this has been established to be coming from that particular feature. (based on Comment 24).
Comment 226 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-24 01:08:52 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10334349&tree=Fx-Team
Comment 234 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-25 10:31:39 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10356498&tree=Firefox
Comment 266 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-26 20:42:16 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10388318&tree=Mozilla-Aurora
Comment 272 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-27 06:08:01 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10402439&tree=Fx-Team
Comment 281 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-27 15:31:48 PDT
https://tbpl.mozilla.org/php/getParsedLog.php?id=10415927&tree=Firefox
Comment 283 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-27 15:40:07 PDT
Should be fixed by bug 738774.
Comment 289 Andrew McCreight [:mccr8] 2012-03-29 07:49:31 PDT
We went an entire day without any of these, so it looks like it is indeed fixed in 14.  Hooray.
Comment 290 Alex Keybl [:akeybl] 2012-03-29 17:40:29 PDT
If we're concerned about the possibility of leaks in user scenarios, please nominate for Aurora landing approval.
Comment 291 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-03-30 03:37:52 PDT
Nominated bug 738774 for aurora-approval. I think we're concerned about these leaks occurring in the wild but not totally sure they are.
Comment 301 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-09 16:02:02 PDT
Please nominate for Aurora uplift.
Comment 302 Phil Ringnalda (:philor) 2012-04-09 16:06:20 PDT
See comment 291, which says it was fixed by bug 738774, which was nominiated and approval-minused, so we're apparently just going to live with these test leaks until 13 falls off the end of the tracks.

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