Hundreds of ghost windows from touch events

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
2 years ago
4 months ago

People

(Reporter: benjamin, Assigned: smaug)

Tracking

(Blocks 2 bugs, {memory-leak, regression})

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox53 unaffected, firefox54+ fixed, firefox55+ fixed)

Details

(Whiteboard: [MemShrink:P1])

Attachments

(5 attachments)

I had some really bad pauses today. I'm not sure which site(s) might be causing this, and I had trouble reading the profile so I'm going to file this and hopefully get some help understanding the profile.

https://perfht.ml/2oPN69Z <- profile
about:memory attached
Questions about profile reading:
There are some really long pauses identified in the upper section with hover titles like "8.NNN second event processing delay on tab thread". But it's not clear to me how to figure out which of the four content processes that is and then how to get the profile of that process but not the rest.
Flags: needinfo?(ehsan)
Talked with Benjamin about this on IRC, these are CC pauses...
Flags: needinfo?(ehsan)
Component: Untriaged → XPCOM
Summary: Really long pauses experienced by bsmedberg → Really long cycle collection pauses experienced by bsmedberg
Based on the memory report, there are 60 ghost windows.
Gecko profiler profiles aren't really useful with leaks.
CC/GC logs from about:memory might reveal the issue.
Yeah, you have hundreds of ghost windows across your various processes. Odds are there is some addon that is causing this.
Blocks: GhostWindows
Keywords: mlk
Summary: Really long cycle collection pauses experienced by bsmedberg → Hundreds of ghost windows
Whiteboard: [MemShrink]
Are you on Firefox 55? What addons do you have installed?
Flags: needinfo?(benjamin)
This was very recent nightly 55 win64. At the time I was running the following:

* Mass Password Reset
* User Agent Switcher
* Moo Later
* Mind the Time

I have since:

installed "Gecko Profiler"
uninstalled "Moo Later" and "Mind the Time"

I will report as I browse today whether I see more ghost windows or similar symptoms.
Flags: needinfo?(benjamin)
Honestly, with that many ghost windows, you were probably leaking every single one you opened. So just browsing for a few minutes and checking about:memory should tell you something. I think I've never seen so many ghost windows in a single memory report, and I've seen a lot of memory reports. ;)
Assignee: nobody → continuation
Summary: Hundreds of ghost windows → Hundreds of ghost windows with addons
Browsing today with the shorter addon list, still seeing ghost windows. memory dump attached
Which "User Agent Switcher" addon do you use? I see 4 with that name and various hypenations.
The most popular one is https://addons.mozilla.org/en-US/firefox/addon/user-agent-switcher/?src=search so I'll try that.
Flags: needinfo?(benjamin)
I'm not sure if it's same issue but I've got 400+ ghost windows
(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #10)
> I'm not sure if it's same issue but I've got 400+ ghost windows

Do you have any of the same addons as listed in comment 6? If not, please file a new bug and needinfo me.
Component: XPCOM → DOM
(In reply to Andrew McCreight [:mccr8] from comment #9)
> Which "User Agent Switcher" addon do you use? I see 4 with that name and
> various hypenations.

Also, do you have any rules or whatever set up for it? I tried browsing a little with the user agent switcher and mass password reset addons installed but I didn't get any leaks. Though I should try it with the profiler, too.
I have the one with ID {e968fc70-8f95-4ab9-9e79-304de2a71ee1}

I'm not actually sure how to tell whether I have any site-specific overrides set up.

Also I don't think I'm using this addon currently, so let me try disabling it as well.
Flags: needinfo?(benjamin)
I'm seeing ghosts locally. I don't know how to reproduce the issue, but atm
the regression range looks to be 
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c697e756f738ce37abc56f31bfbc48f55625d617&tochange=bb38d935d699e0529f9e0bb35578d381026415c4

I got some ghosts with 2017-04-18-10-02-13-mozilla-central (bb38d935d699e0529f9e0bb35578d381026415c4) but so far haven't got using 
2017-04-17-10-02-36-mozilla-central (c697e756f738ce37abc56f31bfbc48f55625d617)
a374c3546993 does seem to leak.
I think I'm seeing this too, is there any information I can provide to help?
STR would be really nice here. I can occasionally reproduce this locally using my main FF profile.
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c697e756f738ce37abc56f31bfbc48f55625d617&tochange=bb38d935d699e0529f9e0bb35578d381026415c4 is probably the right regression range.

Oddly enough https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cac374fb0de1&tochange=a374c3546993 seems to be more precise range, but that doesn't quite make sense.
Could be that I just haven't seen the leak.

Bug 1358979 doesn't seem to help here.
I'm seeing this with no addons so updating the summary.
Summary: Hundreds of ghost windows with addons → Hundreds of ghost windows
(In reply to Olli Pettay [:smaug] from comment #19)
> Oddly enough
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=cac374fb0de1&tochange=a374c3546993 seems to be more
> precise range, but that doesn't quite make sense.

Could bug 1357069 be a candidate?
I haven't opened any pdfs, but perhaps pdf.js accesses all the pages somehow.
I have been experiencing this a lot and also have no addons other than the profiler enabled.

Here's my about:memory (you can see some closed-earlier-today Google Slides and Sheets URLs) and my profile is: https://perfht.ml/2ptKnWU
I'm going through changesets now one by one. Just need to browse random pages after a build is ready, since I still don't have STR, so this takes time.
If anyone has some STR (which might just work locally with a certain FF profile), help would be appreciated.
With my main profile, STR is still:
* launch profile with restored session consisting of 7 pinned tabs
* open a new tab and load something
* close the new tab
(In reply to Olli Pettay [:smaug] from comment #24)
> I'm going through changesets now one by one. Just need to browse random
> pages after a build is ready, since I still don't have STR, so this takes
> time.

Have you run find_roots.py on the CC edges file yet? What does that turn up?

[Tracking Requested - why for this release]: bad leaks many people are seeing
Flags: needinfo?(bugs)
I think I found it. r=smaug patch :p
/me tests.
Flags: needinfo?(bugs)
Assignee: continuation → bugs
(In reply to Olli Pettay [:smaug] from comment #27)
> I think I found it. r=smaug patch :p
> /me tests.

Ugh, nsDocShell::GetPresContext() addrefs!
Posted patch patchSplinter Review
Attachment #8861185 - Flags: review?(bugmail)
[Tracking Requested - why for this release]:
Blocks: 1356695
Keywords: regression
Whiteboard: [MemShrink] → [MemShrink]1356695
Attachment #8861185 - Flags: review?(bugmail) → review+
Thanks for tracking this down, Olli!
Whiteboard: [MemShrink]1356695 → [MemShrink:P1]
Aha, maybe this bug *does* explain why I've been seeing 54.0b1 doing the same thing also then! I really think this should block 54.0b2, Gerry. My experience over the weekend is that this was giving me an extremely janky, nearly unusable browser.
Flags: needinfo?(gchang)
Summary: Hundreds of ghost windows → Hundreds of ghost windows from touch events
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/363b1e7eccc3
nsPresContext is refcounted, so use it so, r=kats
Comment on attachment 8861185 [details] [diff] [review]
patch

(Since I don't know whether one is supposed to ask approval for aurora or beta, asking both.)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1356695
[User impact if declined]: Major leaks
[Is this code covered by automated tests?]: NA
[Has the fix been verified in Nightly?]: NA
[Needs manual test from QE? If yes, steps to reproduce]: Load pages and then
about:memory Measure and look for ghosts
[Is the change risky?]:Not risky
[Why is the change risky/not risky?]: just using refcounted object correctly
[String changes made/needed]:NA
Attachment #8861185 - Flags: approval-mozilla-beta?
Attachment #8861185 - Flags: approval-mozilla-aurora?
Comment on attachment 8861185 [details] [diff] [review]
patch

Let's get this on m-b now, so that we may still be able to build beta 2 tonight.
Attachment #8861185 - Flags: approval-mozilla-beta?
Attachment #8861185 - Flags: approval-mozilla-beta+
Attachment #8861185 - Flags: approval-mozilla-aurora?
Attachment #8861185 - Flags: approval-mozilla-aurora-
Flags: needinfo?(gchang)
https://hg.mozilla.org/mozilla-central/rev/363b1e7eccc3
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Flagging this for manual testing, instructions in Comment 34.
(In reply to Andrei Vaida [:avaida], Desktop Release QA – please ni? me from comment #38)
> Flagging this for manual testing, instructions in Comment 34.

The testing probably needs to be done on a computer with a touch screen, and it requires using the touch screen.
Does need touchscreen, but doesn't need using it. It just needs to be enabled and pages need to do something to try to use touch events or something.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.