Closed
Bug 1451985
Opened 6 years ago
Closed 6 years ago
Ghost windows from dropbox shared image link page
Categories
(Core :: DOM: Core & HTML, defect, P1)
Core
DOM: Core & HTML
Tracking
()
People
(Reporter: bkelly, Assigned: mccr8)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: memory-leak, Whiteboard: [MemShrink:P1])
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
baku
:
review+
jcristau
:
approval-mozilla-esr60+
|
Details |
STR: 1. Open a shared dropbox link like this: https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0 2. Close the tab. 3. Observe that it takes many seconds for the tab to close. I have also gotten a ghost window with dropbox pages like this a few times tonight, but I can't reproduce regularly.
Reporter | ||
Comment 1•6 years ago
|
||
Like this ghost window: 184.11 MB (100.0%) -- explicit ├───64.20 MB (34.87%) -- window-objects │ ├──32.85 MB (17.84%) ++ top(https://www.irccloud.com/irc/freenode/channel/whatwg, id=4294967409) │ ├──14.10 MB (07.66%) -- top(none)/ghost/window(https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0) │ │ ├───9.09 MB (04.94%) ++ js-compartment(https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0) │ │ ├───3.88 MB (02.11%) ── layout/style-sheets │ │ └───1.13 MB (00.61%) ++ (3 tiny) │ ├───8.82 MB (04.79%) ++ top(https://www.fastmail.com/mail/Inbox/7f81296bf1caa8c8-f163802768u10708?u=9509408f, id=4294967407) │ └───8.43 MB (04.58%) ++ top(https://getpocket.com/a/queue/list/, id=4294967424)
Whiteboard: [MemShrink]
Reporter | ||
Comment 2•6 years ago
|
||
Unknown ref on script element: bkelly@valen:/mnt/c/devel/tmp/cclogs$ /srv/heapgraph/find_roots.py cc-edges.9132.1522983791.log 000002804EDEAC00 Parsing cc-edges.9132.1522983791.log. Done loading graph. 0000028061877100 [FragmentOrElement (xhtml) script https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0] --[[via hash] mListenerManager]--> 000002804DB87310 [EventListenerManager] --[mListeners event=onerror listenerType=4 [i]]--> 000002805EE4BD00 [CallbackObject] --[mIncumbentGlobal]--> 000002804EDEAC00 [nsGlobalWindowInner # 4294967610 inner https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0] Root 0000028061877100 is a ref counted object with 1 unknown edge(s). known edges: 00000280697D7AC0 [JS Object (HTMLScriptElement)] --[UnwrapDOMObject(obj)]--> 0000028061877100 000002804DF9D580 [FragmentOrElement (xhtml) head https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0] --[mAttrsAndChildren[i]]--> 0000028061877100
Reporter | ||
Comment 3•6 years ago
|
||
I can see that dropbox script resources are getting alternate bytecode data out of http cache. From about:cache: key: https://cfl.dropboxstatic.com/static/compiled/js/modules/clean/react/file_action_button_type.min-vflOclJZd.js alt-data: 1;580,javascript/moz-bytecode-20180405220026 I'll try disable bytecode cache for a while.
Blocks: GhostWindows
Reporter | ||
Comment 4•6 years ago
|
||
It still happens with bytecode cache disabled: │ ├──39.40 MB (17.51%) -- top(none)/ghost │ │ ├──23.74 MB (10.55%) ++ (16 tiny) │ │ └──15.66 MB (06.96%) -- window(https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0) │ │ ├──10.64 MB (04.73%) -- js-compartment(https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0) │ │ │ ├───8.23 MB (03.66%) -- classes │ │ │ │ ├──3.28 MB (01.46%) ++ class(Object)/objects │ │ │ │ ├──2.78 MB (01.24%) -- class(Function)/objects │ │ │ │ │ ├──2.65 MB (01.18%) ── gc-heap │ │ │ │ │ └──0.13 MB (00.06%) ── malloc-heap/slots │ │ │ │ └──2.17 MB (00.97%) ++ (5 tiny) │ │ │ └───2.41 MB (01.07%) ++ (6 tiny) │ │ ├───3.89 MB (01.73%) ── layout/style-sheets │ │ └───1.13 MB (00.50%) ++ (3 tiny) Note, I also wiped my http cache and restarted to ensure there was not any old bytecode left in use.
Reporter | ||
Comment 5•6 years ago
|
||
Note, opening 10+ dropbox tabs and closing them in various states of loading repeatedly seems to trigger the leak for me.
Reporter | ||
Comment 6•6 years ago
|
||
Note, those 16 "tiny" ghost windows are: │ │ │ ├───1.74 MB (00.77%) ++ window(https://www.google.com/recaptcha/api2/bframe?hl=en&v=v1522045847408&k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&cb=t3vp3yn7ncf5#73ki9ncqqou8) │ │ │ ├───1.74 MB (00.77%) ++ window(https://www.google.com/recaptcha/api2/bframe?hl=en&v=v1522045847408&k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&cb=yq9frk3cu98z#i99y58skfiwd) │ │ │ ├───1.74 MB (00.77%) ++ window(https://www.google.com/recaptcha/api2/bframe?hl=en&v=v1522045847408&k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&cb=xrf5mtq0fu5o#w5naznx65aks) │ │ │ ├───1.73 MB (00.77%) ++ window(https://www.google.com/recaptcha/api2/bframe?hl=en&v=v1522045847408&k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&cb=mspcbydv6efw#yp7yrznxwbe9) │ │ │ ├───1.66 MB (00.74%) ++ window(https://www.google.com/recaptcha/api2/bframe?hl=en&v=v1522045847408&k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&cb=3ef2831omlex#65zr1a9arc31) │ │ │ ├───1.66 MB (00.74%) ++ window(https://www.google.com/recaptcha/api2/bframe?hl=en&v=v1522045847408&k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&cb=8c9cbvpp3iaz#pf2mw33ct60t) │ │ │ ├───1.65 MB (00.73%) ++ window(https://www.google.com/recaptcha/api2/bframe?hl=en&v=v1522045847408&k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&cb=gobjhazg8u4#psoxn0mneq7t) │ │ │ ├───1.59 MB (00.71%) ++ window(https://www.google.com/recaptcha/api2/anchor?k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&co=aHR0cHM6Ly93d3cuZHJvcGJveC5jb206NDQz&hl=en&v=v1522045847408&size=invisible&cb=2ikvx5o7ikmj#i99y58skfiwd) │ │ │ ├───1.59 MB (00.71%) ++ window(https://www.google.com/recaptcha/api2/anchor?k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&co=aHR0cHM6Ly93d3cuZHJvcGJveC5jb206NDQz&hl=en&v=v1522045847408&size=invisible&cb=fprnnppi5swd#w5naznx65aks) │ │ │ ├───1.58 MB (00.70%) ++ window(https://www.google.com/recaptcha/api2/anchor?k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&co=aHR0cHM6Ly93d3cuZHJvcGJveC5jb206NDQz&hl=en&v=v1522045847408&size=invisible&cb=wmjahppxylq#pf2mw33ct60t) │ │ │ ├───1.57 MB (00.70%) ++ window(https://www.google.com/recaptcha/api2/anchor?k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&co=aHR0cHM6Ly93d3cuZHJvcGJveC5jb206NDQz&hl=en&v=v1522045847408&size=invisible&cb=lyypxwa96cli#yp7yrznxwbe9) │ │ │ ├───1.57 MB (00.70%) ++ window(https://www.google.com/recaptcha/api2/anchor?k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&co=aHR0cHM6Ly93d3cuZHJvcGJveC5jb206NDQz&hl=en&v=v1522045847408&size=invisible&cb=lufeylj1i2v3#psoxn0mneq7t) │ │ │ ├───1.57 MB (00.70%) ++ window(https://www.google.com/recaptcha/api2/anchor?k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&co=aHR0cHM6Ly93d3cuZHJvcGJveC5jb206NDQz&hl=en&v=v1522045847408&size=invisible&cb=5j8iuur1tpwi#65zr1a9arc31) │ │ │ ├───1.57 MB (00.70%) ++ window(https://www.google.com/recaptcha/api2/anchor?k=6LdnLyIUAAAAAOiGPtddh-g3KiJRoDGGPD-6dqXo&co=aHR0cHM6Ly93d3cuZHJvcGJveC5jb206NDQz&hl=en&v=v1522045847408&size=invisible&cb=k21lijj5miby#73ki9ncqqou8) │ │ │ ├───0.72 MB (00.32%) ++ window(https://marketing.dropbox.com/s/%3Atkey/%2Apath?referrer=)
Reporter | ||
Updated•6 years ago
|
Summary: dropbox shared image link page is very slow to close (and sometimes leak?) → dropbox shared image link page is very slow to close and sometimes leak
Reporter | ||
Comment 7•6 years ago
|
||
[qf] for the very slow tab close behavior.
Whiteboard: [MemShrink] → [MemShrink][qf]
Updated•6 years ago
|
Whiteboard: [MemShrink][qf] → [MemShrink:P1][qf]
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → continuation
Assignee | ||
Comment 8•6 years ago
|
||
I can reproduce this by shift clicking the link in comment 0 to open about 5 or so tabs, then manually closing them by clicking on the X in the tabs as fast as I can. This seems to only leak one.
Assignee | ||
Comment 9•6 years ago
|
||
In a very slow refcount logging run, I had to wait until the little UI with the log in stuff at the top of the image loaded in before closing the tab. That seemed to make it reproduce more.
Comment 10•6 years ago
|
||
Ben, can you provide a profile for this bug we can do a better analysis.
Depends on: 1344302
Flags: needinfo?(bkelly)
Whiteboard: [MemShrink:P1][qf] → [MemShrink:P1][qf:f64][qf:p3]
Assignee | ||
Comment 11•6 years ago
|
||
(To be clear here, I'm only looking at the leak. I didn't notice any particular slowness.) Using DMD heap scan mode, it looks like ScriptLoadRequest is holding a strong reference to the script element. ScriptLoadRequest is cycle collected, but does not report that field to the CC. Lo and behold, that load request is being held alive by the leaking script element...
Assignee | ||
Comment 12•6 years ago
|
||
I have a patch that fixes the leak for me, but I'll spin this off into a bug blocking this one. I don't know how a leak would cause page closing to be slow, and I haven't noticed that myself.
Assignee | ||
Updated•6 years ago
|
Summary: dropbox shared image link page is very slow to close and sometimes leak → Ghost windows from dropbox shared image link page
Assignee | ||
Comment 13•6 years ago
|
||
Ben suggested I just use this bug for the leak.
Keywords: memory-leak
Assignee | ||
Comment 14•6 years ago
|
||
I audited ScriptLoadRequest and ModuleLoadRequest and it looks like all of the other fields that should be reported to the cycle collector are.
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Comment 16•6 years ago
|
||
I think this bug was around in May 2017 when bug 1362119 landed (and is probably even older), so it likely affects all branches. This looks like a leak Ben and I have been seeing off and on for quite a while, so we should backport this.
Assignee | ||
Updated•6 years ago
|
status-firefox59:
--- → wontfix
status-firefox60:
--- → affected
status-firefox61:
--- → affected
status-firefox-esr52:
--- → wontfix
Assignee | ||
Updated•6 years ago
|
Priority: P3 → P1
Reporter | ||
Updated•6 years ago
|
Status: NEW → ASSIGNED
Flags: needinfo?(bkelly)
Comment 18•6 years ago
|
||
I wonder why this has become an issue only now if the relevant code is after all very old. http://52.25.115.98/viewvc/main/mozilla/content/base/src/nsScriptLoader.cpp?annotate=1.63#l118
Assignee | ||
Comment 19•6 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #18) > I wonder why this has become an issue only now if the relevant code is after > all very old. It is strange. I think Ben and I have been seeing similar leaks for at least a year or two. At least, windows leaking through script elements. See bug 1421681. It does seem to have happening more recently.
Comment 20•6 years ago
|
||
mozreview-review |
Comment on attachment 8969734 [details] Bug 1451985 - Tell the cycle collector about ScriptLoadRequest::mElement. https://reviewboard.mozilla.org/r/238548/#review244536
Attachment #8969734 -
Flags: review?(amarchesini) → review+
Comment 21•6 years ago
|
||
Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/de34f6d415df Tell the cycle collector about ScriptLoadRequest::mElement. r=baku
Assignee | ||
Comment 22•6 years ago
|
||
We're about 2 weeks from the release of 60, and I don't see any evidence on telemetry that this is a wide scale problem, so let's just wont fix it for 60.
Comment 23•6 years ago
|
||
The page is setting a beforeunload event listener, and seems to be busy running a bunch of JS not long after painting, which explains why it's a bit slow to close.
Reporter | ||
Comment 24•6 years ago
|
||
I cloned this over to bug 1456241 to investigate slowness around closing the tab.
See Also: → 1456241
Comment 25•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/de34f6d415df
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee | ||
Comment 26•6 years ago
|
||
Wow, this actually shows up as a distinct drop in ghost windows on Telemetry. The mean content process GHOST_WINDOWS number drops from about 0.3 to more like 0.06 (the other measurements are all 0): https://mzl.la/2ralTRv With the new scalar telemetry MAX_GHOST_WINDOWS (which measures the max value seen across an entire telemetry ping session), the 95th percentile goes from about 1 to 0, and the mean goes from around 0.7 to around 0.3 (the other measurements are all 0): https://mzl.la/2ramGSt
Updated•6 years ago
|
status-firefox-esr60:
--- → affected
Comment 27•6 years ago
|
||
Andrew, do you want to request uplift to esr60?
Flags: needinfo?(continuation)
Assignee | ||
Comment 28•6 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #27) > Andrew, do you want to request uplift to esr60? This doesn't seem to match the esr uplift criteria, as it is not a stability or security fix, and it does not fix a regression.
Flags: needinfo?(continuation)
Comment 29•6 years ago
|
||
This is a simple perf fix and we're very early in the ESR60 lifecycle. I think we should take this.
Flags: needinfo?(continuation)
Assignee | ||
Comment 30•6 years ago
|
||
Comment on attachment 8969734 [details] Bug 1451985 - Tell the cycle collector about ScriptLoadRequest::mElement. [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: see comment 29 User impact if declined: memory leaks, which can lead to performance problems Fix Landed on Version: 61 Risk to taking this patch (and alternatives if risky): Very low. It is a simple patch. String or UUID changes made by this patch: none
Flags: needinfo?(continuation)
Attachment #8969734 -
Flags: approval-mozilla-esr60?
Comment 31•6 years ago
|
||
Comment on attachment 8969734 [details] Bug 1451985 - Tell the cycle collector about ScriptLoadRequest::mElement. fix memory leak, approved for 60.1esr
Attachment #8969734 -
Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Comment 32•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr60/rev/39832495babf
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
Updated•2 years ago
|
Performance Impact: --- → P3
Whiteboard: [MemShrink:P1][qf:f64][qf:p3] → [MemShrink:P1]
You need to log in
before you can comment on or make changes to this bug.
Description
•