Closed Bug 1266993 Opened 8 years ago Closed 8 years ago

irccloud tab leaks when closed

Categories

(Core :: DOM: Core & HTML, defect)

48 Branch
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox48 --- affected

People

(Reporter: bkelly, Assigned: dbaron)

References

Details

(Whiteboard: [MemShrink:P2] btpp-followup-2016-05-06)

Attachments

(3 files)

+++ This bug was initially created as a clone of Bug #1266915 +++

Split out the irccloud tab leak from bug 1266915 comment 2.

│  ├────359.26 MB (18.13%) -- top(none)/detached
│  │    ├──255.53 MB (12.89%) -- window(https://irccloud.mozilla.com/#!/ircs://irc1.dmz.scl3.mozilla.com:6697/%23content)
│  │    │  ├──176.03 MB (08.88%) ++ js-compartment(https://irccloud.mozilla.com/#!/ircs://irc1.dmz.scl3.mozilla.com:6697/%23content)
│  │    │  ├───75.99 MB (03.83%) ++ dom
│  │    │  └────3.51 MB (00.18%) ++ (3 tiny)
For the instance that happened today there are also two ghost windows for stripe:

│  ├───80.72 MB (08.11%) -- top(none)
│  │   ├──80.14 MB (08.05%) -- detached
│  │   │  ├──76.88 MB (07.72%) ++ window(https://irccloud.mozilla.com/#!/ircs://irc1.dmz.scl3.mozilla.com:6697/%23content)
│  │   │  └───3.26 MB (00.33%) ++ (3 tiny)
│  │   └───0.58 MB (00.06%) -- ghost
│  │       ├──0.29 MB (00.03%) ++ window(https://js.stripe.com/v2/channel.html?stripe_xdm_e=https%3A%2F%2Firccloud.mozilla.com&stripe_xdm_c=default346270&stripe_xdm_p=1#__stripe_transport__)
│  │       └──0.29 MB (00.03%) ++ window(https://js.stripe.com/v2/channel.html?stripe_xdm_e=https%3A%2F%2Firccloud.mozilla.com&stripe_xdm_c=default694076&stripe_xdm_p=1#__stripe_transport__)
I haven't been able to reproduce this since yesterday afternoon.  I think it might be related to not getting any notifications from irccloud in that time frame.  So maybe this is related to bug 1266912.
See Also: → 1266912
When you get these leaks, please create CC/GC logs, both concise and verbose.
I have the concise CC logs from the first instance.  I sent them to Andrew.  I will forward to you.
I got a notification this morning from irccloud.  I closed the window and it leaked.  I believe the alert.xul detached window got bigger as well.

So I really think notifications are leaking windows somehow.
I think I see why alert.xul is leaking in bug 1266912 comment 2.  I'll see if that helps with this bug as well.
See Also: → 1266856
Whiteboard: [MemShrink] → [MemShrink] btpp-followup-2016-05-06
Whiteboard: [MemShrink] btpp-followup-2016-05-06 → [MemShrink:P2] btpp-followup-2016-05-06
So I was seeing 8 second (!) cycle collection pauses today.  Based on the CC logs it appears to be due to this bug.  find_roots.py says:

0x22d08dd0 [Notification https://irccloud.mozilla.com/#!/ircs://irc1.dmz.scl3.mozilla.com:6697/%23content]
    --[Preserved wrapper]--> 0x7f363ba19250 [JS Object (Notification)]
    --[global]--> 0x7f36522571a0 [JS Object (Window)]
    --[SESSION]--> 0x7f364891c740 [JS Object (Object)]
    --[group_proto]--> 0x7f3648961600 [JS Object (Object)]
    --[postMessages]--> 0x7f3622617400 [JS Object (Array)]
    --[objectElements[0]]--> 0x7f364aa45a60 [JS Object (MessageEvent)]
    --[UnwrapDOMObject(obj)]--> 0x152f0890 [Event]
    --[mWindowSource]--> 0x15a344e0 [nsGlobalWindow # 2147484258 inner https://js.stripe.com/v2/channel.html?stripe_xdm_e=https%3A%2F%2Firccloud.mozilla.com&stripe_xdm_c=default594599&stripe_xdm_p=1#__stripe_transport__]

    Root 0x22d08dd0 is a ref counted object with 1 unknown edge(s).
    known edges:
       0x7f363ba19250 [JS Object (Notification)] --[UnwrapDOMObject(obj)]--> 0x22d08dd0



or, looking for a different root:

0x22d08dd0 [Notification https://irccloud.mozilla.com/#!/ircs://irc1.dmz.scl3.mozilla.com:6697/%23content]
    --[Preserved wrapper]--> 0x7f363ba19250 [JS Object (Notification)]
    --[global]--> 0x7f36522571a0 [JS Object (Window)]
    --[UnwrapDOMObject(obj)]--> 0x31f4f50 [nsGlobalWindow # 2147484255 inner https://irccloud.mozilla.com/#!/ircs://irc1.dmz.scl3.mozilla.com:6697/%23content]

    Root 0x22d08dd0 is a ref counted object with 1 unknown edge(s).
    known edges:
       0x7f363ba19250 [JS Object (Notification)] --[UnwrapDOMObject(obj)]--> 0x22d08dd0
David, I haven't seen this since I removed the mozilla-tree-status add-on.  Do you have any addons enabled?
Flags: needinfo?(dbaron)
I don't have addons, and we figured out what was wrong here.

First of all:  my usage pattern:  I have IRCCloud in an app tab.  But since IRCCloud isn't great at showing stuff that was missed when my laptop is suspended, I occasionally reload it by reentering the URL in that pinned tab.  This might matter, although I tend to think it doesn't.

Second, the underlying problem with the leak is related to notifications, which khuey and I just debugged (using CC logs and an rr recording in which I reproduced the bug, which is reproducable at will -- get an IRC notification and then reload IRCCloud in that tab, and the child process will have leaked stuff from the old irccloud).

IRCCloud sends notifications that have an Icon; the code for dealing with such notifications seems to be broken for GNOME.

In particular, the code in dom/notification/Notification.cpp has a reference that gets cleared by MainThreadNotificationObserver::Observe("alertfinished") being called (though Kyle couldn't quite say how this works).  The GNOME alert code has the bug that it sometimes never finishes the alert process because it has a listener that gets destroyed because nothing is holding a reference to it.  In particular, the irccloud notifications load the image https://irccloud.mozilla.com/static/80x80-ent.png , and the GNOME alerts code expects nsAlertsIconListener::Notify to be called with LOAD_COMPLETE and then with FRAME_COMPLETE.  We get LOAD_COMPLETE, but then never get FRAME_COMPLETE because the destructor gets called first because nobody is holding a reference.  And it's FRAME_COMPLETE that was supposed to call nsAlertsIconListener::ShowAlert which would actually show the alert, which would in turn allow me to close the GNOME alert notification.

(The GNOME nsAlertsIconListener code keeps itself alive while *GNOME* is showing the alert, but it doesn't keep itself alive for the image load.)
Flags: needinfo?(dbaron)
My steps to reproduce:

1. Set up a clean profile that's logged in to IRCCloud, and accept sending of notifications (which might require somebody mentioning you)
2. open two tabs in that profile, one with IRCCloud and the other with example.com (to prevent the child process from being killed)
3. have somebody mention your name on IRC (might require IRCCloud tab in the background?)
4. switch to the IRCCloud tab, and reload it by removing the part of the URL after the "#" and then hitting enter
5. go to about:memory, press "Measure", and scroll down to the child process part (little blue down arrow), and look under
explicit -> window objects, collapse the open window, and look under "tiny", and in that under "top(none)/detached"

Actual results:  detached windows are still around:

185.04 MB (100.0%) -- explicit
├──129.20 MB (69.82%) -- window-objects
│  ├──127.81 MB (69.07%) ++ top(https://irccloud.mozilla.com/#!/ircs://irc1.dmz.scl3.mozilla.com:6697/%23content, id=2147483649)
│  └────1.39 MB (00.75%) -- (3 tiny)
│       ├──0.93 MB (00.50%) -- top(none)/detached
│       │  ├──0.33 MB (00.18%) ++ window(https://staticxx.facebook.com/connect/xd_arbiter.php?version=42#channel=f949b40cfc26e&origin=https%3A%2F%2Firccloud.mozilla.com)
│       │  ├──0.31 MB (00.17%) ++ window(about:blank)
│       │  ├──0.29 MB (00.16%) ++ window(https://js.stripe.com/v2/channel.html?stripe_xdm_e=https%3A%2F%2Firccloud.mozilla.com&stripe_xdm_c=default801244&stripe_xdm_p=1#__stripe_transport__)
│       │  └──0.00 MB (00.00%) ── window(https://irccloud.mozilla.com/#!/ircs://irc1.dmz.scl3.mozilla.com:6697/%23content)/dom/other
│       ├──0.29 MB (00.16%) ++ top(http://example.com/, id=2147483651)
│       └──0.17 MB (00.09%) ++ top(about:blank, id=2147483672)
This both causes alerts that involve icons to actually show up on GNOME,
and prevents those alerts from being kept alive and leaking the window
that contains them.

MozReview-Commit-ID: HgSAtbcvogO
Attachment #8765090 - Flags: review?(seth)
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
With this patch, I both:

 * see an actual notification when somebody mentions me on IRC!

 * don't leak using the steps in comment 12
Awesome!  The original report here was for Windows or Mac, but happy to repurpose it for Linux.  I think my original problem is captured in a bug about addon leaks now.
Comment on attachment 8765090 [details] [diff] [review]
Make nsAlertsIconListener keep itself alive while it's expecting notifications from the icon load

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

Unfortunately Kit just rewrote this whole thing; this patch won't apply on central. Given the rewrite, we should check if the bug is already fixed.
Attachment #8765090 - Flags: review?(seth)
Kit rewrote it in bug 1233086, by the way.
Even if this is fixed on central we should consider taking this on branches.
bug 1233086, without this patch, fixes the leaks and the notifications (comment 12, although tested with actual use rather than a clean profile)
(In reply to Ben Kelly [:bkelly] from comment #8)
> David, I haven't seen this since I removed the mozilla-tree-status add-on. 
> Do you have any addons enabled?

Let's call this bug worksforme, then.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Or maybe we should figure out what's happening with the mozilla-tree-status addon?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: