Ghost windows on Gmail and washingtonpost.com

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
5 months ago

People

(Reporter: mccr8, Assigned: baku)

Tracking

(Blocks 1 bug, {memory-leak, regression})

Trunk
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

(Whiteboard: [MemShrink:P1])

Attachments

(1 attachment, 1 obsolete attachment)

I have a bunch of ghost windows from Google plus.

Here's the start of the path entraining one of the nsGlobalWindows:

0x136659480 [DOMEventTargetHelper https://plus.google.com/u/1/_/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%3Dgapi.gapi.en.6zwg_pEP_2A.O%2Fm%3D__features__%2Fam%3DAAQ%2Frt%3Dj%2Fd%3D1%2Frs%3DAItRSTNy-00ELgURzlKy-V-78pEvv5BpHA#pid=23&rpctoken=424140231&_methods=onError%2ConInfo%2ChideNotificationWidget%2CpostSharedMessage%2Creauth%2CsetNotificationWidgetHeight%2CsetNotificationWidgetSize%2CswitchTo%2CnavigateTo%2CsetNotificationText%2CsetNo]
    --[mListenerManager]--> 0x1392b2310 [EventListenerManager]
    --[mListeners event=onmessage listenerType=1 [i]]--> 0x13abdc970 [JSEventHandler handlerName=onmessage]
    --[mTypedHandler.Ptr()]--> 0x13abdc940 [CallbackObject]
    --[mCallback]--> 0x1616a5700 [JS Object (Function - Mda/b.port1.onmessage)]
    --[group_function, function]--> 0x164a96ec0 [JS Object (Function - Mda/b.port1.onmessage)]

    Root 0x136659480 is a ref counted object with 1 unknown edge(s).
    known edges:
       0x135f63580 [MessageChannel] --[mPort1]--> 0x136659480

I'll see if I can figure out how to reproduce it. You have to look for window/detached because until you close gmail it won't be treated as a ghost window.
Assignee: nobody → continuation
Keywords: mlk
Whiteboard: [MemShrink]
Just curious, is this e10s or non-e10s?

Though, that probably doesn't matter.
baku, could you take a look at this?

What happens to MessagePorts between windowA and windowB when windowA is closed?
Flags: needinfo?(amarchesini)
When WindowA is closed, MessagePort receives a inner-window-destroyed notification and it calls Close().
At this point we send a PBackground message to close the other side and we release the MessagePort.
When the port hosted in windowB receives the Close(), we release that port.
Similar story if we have a worker. And we have some mochitest for this.

Would be nice if we can reproduce it somehow. Maybe we can debug it together today.
Flags: needinfo?(amarchesini)
I found a way to consistently reproduce something that looks like this:
1. Go to washingtonpost.com, scroll down to the bottom so it all starts loading up, then up to the top.
2. Click on a news story, scroll down to the bottom then back up to the top.
3. While it is still loading, close the tab.

This causes both washingtonpost.com and the news story pages to be leaked.
Now that there is STR maybe baku could investigate. :)
Assignee: continuation → nobody
Flags: needinfo?(amarchesini)
I can reproduce this both with and without e10s.

[Tracking Requested - why for this release]: severe memory leaks
Summary: Ghost windows on plus.google.com → Ghost windows on Gmail and washingtonpost.com
Blocks: 952139
Keywords: regression
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Posted patch mp2.patch (obsolete) — Splinter Review
The issue is that, when 1 MessageChannel is created, 2 MessagePorts are created too and they know each other to optimize postMessages in the same window.
They both are observers for inner-window-destroy.
When the window is closed, the first port receives the notification and it close itself and the other port. At this point we forget to call RemoveObserver.

This patch does:
1. RemoveObserver is called before call the manual Release(). Note: MessagePort needs to do this because we don't have multi-thread CC.
2. ::Observe() must be received only when the MessagePort is keeping itself alive.
Attachment #8626922 - Flags: review?(continuation)
Posted patch mp2.patchSplinter Review
Attachment #8626922 - Attachment is obsolete: true
Attachment #8626922 - Flags: review?(continuation)
Attachment #8626924 - Flags: review?(continuation)
Comment on attachment 8626924 [details] [diff] [review]
mp2.patch

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

Sounds reasonable to me. Thanks for the quick fix.
Attachment #8626924 - Flags: review?(continuation) → review+
https://hg.mozilla.org/mozilla-central/rev/de8d49d52430
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Duplicate of this bug: 1178773
I'm going to retroactively mark this as a MemShrink P1.
Whiteboard: [MemShrink] → [MemShrink:P1]
Duplicate of this bug: 1179928
Adding a tracking flag for FF42 as this a memory leak issue.
Andrew, I am a little confused whether the fix for this bug is in FF42 or not. This landed on 6/29 in m-c, I believe that was FF41 at that point in time. So, is this fix in 41 and 42 both? Or just 41?
Flags: needinfo?(continuation)
That's odd. I guess I got confused somehow. You are right, it looks like landed on m-c when it was 41, so 42 shouldn't be affected.

Here's the patch in Aurora and m-c:
https://hg.mozilla.org/releases/mozilla-aurora/rev/de8d49d52430
https://hg.mozilla.org/mozilla-central/rev/de8d49d52430
Flags: needinfo?(continuation)
Blocks: GhostWindows
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.