devtools memory panel leaks windows on sites using iframes with postMessage

RESOLVED WORKSFORME

Status

P1
normal
RESOLVED WORKSFORME
2 years ago
2 months ago

People

(Reporter: bkelly, Unassigned)

Tracking

({stale-bug})

47 Branch
stale-bug

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
I updated to FF47 earlier today through a manual update.  Within a few hours I had leaked windows in my gmail:

├───72.52 MB (13.54%) -- window-objects
│   ├──56.73 MB (10.59%) -- top(none)
│   │  ├──56.73 MB (10.59%) -- ghost
│   │  │  ├──20.01 MB (03.73%) ++ window(https://mail.google.com/_/scs/mail-static/_/js/k=gmail.main.en.r-EglRaxyPc.O/m=m_i,t/am=nhEPBNzJuD8Y1xgFyEhfoTDvveezJeVHTvCi_oQBEKlVAP43-38Anwd60RYK/rt=h/d=1/rs=AHGWq9AoKxMAwHsmyZCOJzmvVPSDASMtrg)
│   │  │  ├──15.80 MB (02.95%) ++ window(https://mail.google.com/mail/u/0/#label/-BZ-Watch/1552d58e49e27fab)
│   │  │  ├──14.64 MB (02.73%) ++ window(https://mail.google.com/mail/u/0/#inbox)
│   │  │  └───6.28 MB (01.17%) ++ window(https://plus.google.com/u/0/_/notifications/frame?sourceid=23&hl=en&origin=https%3A%2F%2Fmail.google.com&uc=1&jsh=m%3B%2F_%2Fscs%2Fabc-static%2F_%2Fjs%2Fk%3

I have the CC logs, but they are not very helpful:

$ /c/devel/heapgraph/find_roots.py cc-edges.11180.1465344993.log 0000018A95FCC800
Parsing cc-edges.11180.1465344993.log. Done loading graph.

0000018D05172CE0 [JS Object (Window)]
    --[UnwrapDOMObject(obj)]--> 0000018A96228400 [nsGlobalWindow # 1789 inner https://mail.google.co
m/_/scs/mail-static/_/js/k=gmail.main.en.r-EglRaxyPc.O/m=m_i,t/am=nhEPBNzJuD8Y1xgFyEhfoTDvveezJeVHTv
Ci_oQBEKlVAP43-38Anwd60RYK/rt=h/d=1/rs=AHGWq9AoKxMAwHsmyZCOJzmvVPSDASMtrg]
    --[mOuterWindow]--> 0000018A95FCC800 [nsGlobalWindow # 1782 outer ]

    Root 0000018D05172CE0 is a marked GC object.


bkelly@kosh /c/devel/tmp/cclogs/gmail
$ /c/devel/heapgraph/find_roots.py gc-edges.11180.1465344993.log -bro 0000018D05172CE0
Parsing gc-edges.11180.1465344993.log. Done loading graph.

Didn't find a path.
(Reporter)

Comment 1

2 years ago
Created attachment 8761011 [details]
memory-report.json.gz
(Reporter)

Comment 2

2 years ago
Andrew, do you know why find_roots.py would give me "Didn't find a path" for the gc edges with -bro?
Flags: needinfo?(continuation)
(Reporter)

Comment 3

2 years ago
There is an event target still on one of the windows:

   ├─────72 (05.57%) -- top(none)/ghost
   │     ├──24 (01.86%) -- window(https://mail.google.com/mail/u/0/#label/-BZ-Watch/1552d58e49e27fab)/dom
   │     │  ├──23 (01.78%) ── event-listeners
   │     │  └───1 (00.08%) ── event-targets
(Reporter)

Comment 4

2 years ago
Ok, I retraced my steps and figured out steps to reproduce.  I believe this is a leak caused by devtools memory panel.  I haven't used this panel much before, so that's why I'm just seeing it now.

Since this panel is not something most users are seeing I'm dropping the tracking flag for 47.  Since this is probably a bug thats been in the memory panel from the beginning I'm dropping the regression flag.

STR:

1) Open new browser
2) Open gmail in a tab (or another site using iframes for postMessaging like twitter)
3) Open devtools
4) Switch to memory panel (you may have to enable it in your devtools options)
5) Check "record allocation stacks"
6) Take a snapshot
7) Close the gmail tab
8) Open about:memory in a new tab
9) Minimize and measure.  You should see detched or ghost windows for the closed tabs.

I'm not sure why, but this does not trigger for simpler sites AFAICT.
tracking-firefox47: ? → ---
Component: DOM → Developer Tools: Memory
Keywords: regression
Product: Core → Firefox
Summary: FF47 is leaking ghost windows in gmail → devtools memory panel leaks windows on sites using iframes with postMessage
Version: 48 Branch → 47 Branch
(Reporter)

Comment 5

2 years ago
I see JS Object (SavedFrame) in the CC log, but still can't find a path to the root.
(Reporter)

Comment 6

2 years ago
Note, clicking the trash can icon to drop the snapshots before closing devtools and the window does not prevent the leak.

Comment 7

2 years ago
Flagging fitzgen, for memory panel.
Flags: needinfo?(nfitzgerald)

Comment 8

2 years ago
SavedFrame objects are retained by the per-object metadata weakmap (JSCompartment::objectMetadataTable), which associates arbitrary JS objects with their allocation site stacks. The stacks are themselves JSObjects, but they're memoized and reused, so if you have a lot of allocations at the same call stack, all the objects will share the same stack.

I'm not sure why they're being retained, though.
(In reply to Ben Kelly [:bkelly] from comment #2)
> Andrew, do you know why find_roots.py would give me "Didn't find a path" for
> the gc edges with -bro?

You should run find_roots on the CC graph first. Though it sound like that didn't help either. Maybe debugger roots aren't logged in the GC graph? I haven't look at debugger roots much.
Flags: needinfo?(continuation)
(Reporter)

Comment 10

2 years ago
(In reply to Andrew McCreight [:mccr8] from comment #9)
> You should run find_roots on the CC graph first. Though it sound like that
> didn't help either. Maybe debugger roots aren't logged in the GC graph? I
> haven't look at debugger roots much.

I did run find_roots on the CC log first.  I included that in the output in comment 0.  I then took the JS Object (Window) it found and tried to find a root in GC log.  Thats when it couldn't find a path.

Comment 11

2 years ago
What is a "debuggeer root"? The Debugger API itself doesn't root anything, as far as I know.

SavedFrame objects are retained (effectively) by the objects they're the metadata for, via a weakmap. It's no different from any other weakmap edges.
(Reporter)

Comment 12

2 years ago
Just to clarify, I don't know that the SavedFrame objects are a problem.  I was just trying to find find some thread to pull in the logs since find_roots wasn't giving me anything.
In bug 1261869, I was able to make a simple mochitest that demonstrates a (the?) leak just by opening devtools and refreshing. I am pretty sure that at least that leak is unrelated to the memory panel, and that we only noticed it once we got tools that highlighted the problem (but I'm not 100% certain). Unfortunately, that leak seems to be coming into the GC heap from some CC->GC edge, so JS::ubi::Node can't see it. I meant to dig in with the find_roots.py script, but haven't found the time.
Flags: needinfo?(nfitzgerald)
Priority: -- → P1

Comment 14

2 years ago
A memory devtool leaking memory is bad, lets P1 for now. fitzgen indicated this might be a dup of 1261869.
Whiteboard: [MemShrink] → [MemShrink:P1]
As per the DevTools triage process doc [1], a P1 is defined as:
- a security hole
- causing data-loss
- causing a crash
- stopping the tools working
- embarrassing
Should be fixed within 24h and your manager should know about it and, between you, assign it to someone who can fix it.

This bug has been set to P1 more than 4 months ago and is still unassigned.

Nick: who do you think is a good person in devtools to look after this P1 bug now? Alternatively, do you think it should be demoted to P2?

[1] https://docs.google.com/document/d/1uG0foc0pphXJB489_8ClKjYr1wbRXeFyrkd_kIOv9ao/edit#
Flags: needinfo?(nfitzgerald)
(In reply to Patrick Brosset <:pbro> from comment #15)
> As per the DevTools triage process doc [1], a P1 is defined as:
> - stopping the tools working
> - embarrassing

Both of these apply. I think we need to re-test because we did fix some other leaking windows caused by devtools that *maybe* cleared this up as well.

> Nick: who do you think is a good person in devtools to look after this P1
> bug now? Alternatively, do you think it should be demoted to P2?

I think jimb or gregtatum are the right people.
Flags: needinfo?(nfitzgerald)
jimb, would you have tim to re-test this and confirm that this issue still exists and still needs to be a P1 (in which case it needs to be assigned to someone). See comment 15 and comment 16.
Flags: needinfo?(jimb)

Comment 18

2 years ago
Yes, I can do this.
Flags: needinfo?(jimb)

Comment 20

11 months ago
(In reply to Jim Blandy :jimb from comment #18)
> Yes, I can do this.

Jim do you know if this was ever done?
Flags: needinfo?(jimb)
Whiteboard: [MemShrink:P1] → [MemShrink:P2]

Comment 21

11 months ago
The steps in comment #4 no longer show any ghost windows, in either the parent or the content process, regardless of whether we delete the snapshot or close devtools before closing the gmail tab.

Closing as WORKSFORME.
Status: NEW → RESOLVED
Last Resolved: 11 months ago
Flags: needinfo?(jimb)
Resolution: --- → WORKSFORME

Updated

2 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.