Last Comment Bug 1150127 - Leaking windows on Nightly
: Leaking windows on Nightly
Status: VERIFIED FIXED
[MemShrink]
: mlk, regression
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
-- normal with 1 vote (vote)
: mozilla40
Assigned To: Seth Fowler [:seth] [:s2h]
:
: Milan Sreckovic [:milan]
Mentors:
: 1151276 (view as bug list)
Depends on:
Blocks: 1137037
  Show dependency treegraph
 
Reported: 2015-04-01 10:55 PDT by Andrew McCreight [:mccr8]
Modified: 2015-04-10 08:56 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
fixed
+
verified


Attachments
find_roots report for a TechCrunch window (5.86 KB, text/plain)
2015-04-01 10:59 PDT, Andrew McCreight [:mccr8]
no flags Details
Stop leaking windows via imgCacheValidator (3.12 KB, patch)
2015-04-06 19:35 PDT, Seth Fowler [:seth] [:s2h]
amarchesini: review+
lhenry: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Andrew McCreight [:mccr8] 2015-04-01 10:55:38 PDT
Steps to reproduce:
1. Open www.techcrunch.com
2. Scroll down the page.
3. Close TechCrunch.
4. Wait. (Maybe not needed.)
5. Go to about:memory, click on "Measure".  There's a non-zero value for ghost-windows in the child process, which means we're leaking windows.

I'm not sure if this reproduces in a clean profile, but I don't think I have many addons installed.
Comment 1 User image Andrew McCreight [:mccr8] 2015-04-01 10:59:43 PDT
Created attachment 8586886 [details]
find_roots report for a TechCrunch window

The window is being held alive by the TechCrunch document, via some onvisibilitychange listener.  I don't know what the missing reference to the TechCrunch document is.
Comment 2 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2015-04-01 13:44:21 PDT
I can't reproduce this locally, but perhaps there are some US specific ads on the site, or even
mccr8 specific ads.
Comment 3 User image Andrew McCreight [:mccr8] 2015-04-04 19:01:50 PDT
*** Bug 1151276 has been marked as a duplicate of this bug. ***
Comment 4 User image Andrew McCreight [:mccr8] 2015-04-04 19:04:28 PDT
Andreas is seeing this, too, so it isn't just limited to me.  Telemetry for GHOST_WINDOWS doesn't seem to show any particular increase, but my local telemetry that has a bunch of them says 0, so I guess it is broken somehow.  I'll file a bug for that.  My guess is there's some e10s issue there.
Comment 5 User image Guilherme Lima 2015-04-04 20:52:10 PDT
I'm also suffering from ghost windows on Nightly. I have e10s off. All my ghost windows are from facebook.com
I don't have specific steps to reproduce, just browse around and make Firefox use a lot of memory.
The last time it happened, I used google maps/street view a lot (with facebook open on another tab), and after that, Nightly never released the memory used.
Comment 6 User image Andrew McCreight [:mccr8] 2015-04-06 18:12:04 PDT
I bisected it down locally to this push by Seth:
  https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2e9b6048bd0f&tochange=f687743cc499

The easiest way I found to reproduce it was to load techcrunch.com, then quickly close the tab while it was still loading.  It seems plausible that something goes awry that keeps the window alive when the page is closed in the middle of things.

I did have a little bit of trouble reproducing this in a fresh profile, so if this isn't enough to go on, Seth, I can do some local builds to narrow it down further.
Comment 7 User image Andrew McCreight [:mccr8] 2015-04-06 18:12:41 PDT
[Tracking Requested - why for this release]: Severe memory leaks that also result in severe pauses of a second or more.
Comment 8 User image Seth Fowler [:seth] [:s2h] 2015-04-06 18:18:16 PDT
I'd bet it's bug 1137037. Presumably something is keeping alive the imgCacheValidator, which now contains an nsCOMPtr to the document. Maybe the channel/request, via the ImageLib cache?
Comment 9 User image Seth Fowler [:seth] [:s2h] 2015-04-06 19:28:49 PDT
I hate to say it, but I can't reproduce this locally either. I feel pretty confident about the cause of the issue, though. Andrew, hopefully you can verify the fix.
Comment 10 User image Seth Fowler [:seth] [:s2h] 2015-04-06 19:35:13 PDT
Created attachment 8588876 [details] [diff] [review]
Stop leaking windows via imgCacheValidator

Here's the patch. We make sure to free imgCacheValidator::mContext in
OnStartRequest. I don't think the API allows it, but just in case we ever only
get OnStopRequest, I free it there too.
Comment 11 User image Seth Fowler [:seth] [:s2h] 2015-04-06 19:36:44 PDT
https://treeherder.mozilla.org/#/jobs?repo=try&revision=921553944db6
Comment 12 User image Andrew McCreight [:mccr8] 2015-04-06 19:37:51 PDT
Thanks for looking at this so quickly.  I'll check tomorrow.
Comment 13 User image Andrew McCreight [:mccr8] 2015-04-07 07:34:10 PDT
I tried a few times and wasn't able to reproduce it with your patch.
Comment 14 User image Ryan VanderMeulen [:RyanVM] 2015-04-07 08:01:52 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e7488db2bd3

Maybe we'll get lucky and this will help with some OOM-looking issues we're having in automation too.
Comment 15 User image Seth Fowler [:seth] [:s2h] 2015-04-07 11:37:01 PDT
Thanks for the review, verification, and push, everyone!
Comment 16 User image Ryan VanderMeulen [:RyanVM] 2015-04-07 13:05:31 PDT
https://hg.mozilla.org/mozilla-central/rev/a93fb336e91e
Comment 17 User image Seth Fowler [:seth] [:s2h] 2015-04-07 13:17:22 PDT
Comment on attachment 8588876 [details] [diff] [review]
Stop leaking windows via imgCacheValidator

We need this on Aurora.

Approval Request Comment
[Feature/regressing bug #]: Bug 1137037.
[User impact if declined]: Possibly major memory leaks.
[Describe test coverage new/current, TreeHerder]: On m-c.
[Risks and why]: Very low risk.
[String/UUID change made/needed]: None.
Comment 18 User image Liz Henry (:lizzard) (needinfo? me) 2015-04-09 08:48:12 PDT
Florin can you verify this fix on Nightly based on the STR? Thanks! 
I'd like to give it another day or so on Nightly for it to bake before uplift.
Comment 19 User image Andrew McCreight [:mccr8] 2015-04-09 10:34:25 PDT
The problem seems fixed for me in Nightly.

Closing the tab while the page is still loading seems to help trigger it.
Comment 20 User image Petruta Rasa [QA] [:petruta] 2015-04-10 00:24:01 PDT
We couldn't reproduce using Nightly 40.0a1 2015-04-01 under Win 7 64-bit and Mac OS X 10.9.5.

Since for Andrew it's correctly working now (comment 19), I'm marking this as verified.
Comment 21 User image Liz Henry (:lizzard) (needinfo? me) 2015-04-10 08:13:19 PDT
Comment on attachment 8588876 [details] [diff] [review]
Stop leaking windows via imgCacheValidator

Approving for uplift to aurora.
Comment 22 User image Ryan VanderMeulen [:RyanVM] 2015-04-10 08:56:10 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/6d6a22b769bd

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