Closed
Bug 630947
Opened 14 years ago
Closed 14 years ago
reloading about:home builds up 50+ DOMWindows
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
DUPLICATE
of bug 630932
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: sayrer, Assigned: smaug)
References
Details
(Whiteboard: [hardblocker][has patch])
Attachments
(1 file)
(deleted),
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
We seem to be hanging on to references. Nothing leaks at shutdown. Opening a new browser window and closing the old one does not collect these references.
Reporter | ||
Updated•14 years ago
|
blocking2.0: --- → betaN+
OS: Windows CE → All
Whiteboard: [hardblocker]
Comment 1•14 years ago
|
||
I just tried that (on a not-quite-tip mac build).
I load the page, hold down the reload key. Get about 100 windows. Open a new window. Close the old one. Not much change. Load about:blank in the new window. All the windows get GCed....
Reporter | ||
Comment 2•14 years ago
|
||
does your build have this changeset?
Andreas Gal - Enter compartment in AutoScriptEvaluate (bug 630243, r=jst, a=blocker).
?
Comment 3•14 years ago
|
||
No. My build is from Jan 28 with some local changes.
Comment 4•14 years ago
|
||
Sigh of relief ...
Comment 5•14 years ago
|
||
Is this related to the expando leak peterv just fixed?
Comment 6•14 years ago
|
||
Doesn't look like it is.
There's an event listener held by a div element, we're missing an edge:
0x2046eb98 [nsXPCWrappedJS (nsIDOMEventListener)]
0x20456060 [nsGenericElement (xhtml) div [1/2]]
--[[via hash] mListenerManager]-> 0x2046ebd0 [nsEventListenerManager]
--[mListeners[i] mListener]-> 0x2046eb98 [nsXPCWrappedJS (nsIDOMEventListener)]
known edges:
0x20455fe0 nsGenericElement (xhtml) div --[mAttrsAndChildren[i]]--> 0x20456060
Comment 7•14 years ago
|
||
Cc:ing smaug in case the event listener stuff above rings any bells.
Assignee | ||
Comment 8•14 years ago
|
||
Who knows about about:home implementation?
It seems to be a mix of that xhtml+js page and some logic in browser.js
(And no, a missing edge to XPCWrappedJS DOMEventListener doesn't ring any bells)
Ccing some of the people from the recent about:home changes.
Assignee | ||
Comment 10•14 years ago
|
||
Actually, I'm not sure I can reproduce the problem.
Sure, if I press ctrl+r, lots of new DOMWindows are created and kept alive some
time. But once GC and CC have both run, extra DOMWindows are deleted.
Though, I think I managed to reproduce the problem yesterday :/
Today I'm not getting that "restore session" thing in the page.
It is not clear to me when that is shown.
Comment 11•14 years ago
|
||
(In reply to comment #10)
> Though, I think I managed to reproduce the problem yesterday :/
> Today I'm not getting that "restore session" thing in the page.
> It is not clear to me when that is shown.
on load of the page browser checks if sessionstore canRestoreSession and flips hidden status of a div.
Comment 12•14 years ago
|
||
see BrowserOnAboutPageLoad
Assignee | ||
Comment 13•14 years ago
|
||
Ok, now I know how to get that "Restore previous session" and that doesn't
seem to affect the behavior. DOMWindows are collected eventually, at least
on my debug linux build.
But that missing edge is still strange.
Assignee | ||
Comment 14•14 years ago
|
||
Perhaps page loading should more likely call gc+cc (similarly to what happened
before bug 624549). Testing a patch...
Assignee | ||
Comment 15•14 years ago
|
||
This should release DOMWindows earlier.
With the patch we end up calling GC more often;
morethe way we used to do it. If MaybeCC ends up calling CC, GC+CC is called
after page loads and when user becomes inactive.
But I still wonder, can anyone actually reproduce possible leak?
What if you reload about:home 100 times?
Comment 16•14 years ago
|
||
I can reproduce and GC and CC run periodically, we just don't collect because of missing edges.
Comment 17•14 years ago
|
||
Looks like the problem is that XPCJSRuntime::SuspectWrappedNative calls GetFlatJSObjectAndMark instead of GetFlatJSObjectNoMark :-/. I'll verify in the morning.
Updated•14 years ago
|
Assignee: nobody → peterv
Comment 18•14 years ago
|
||
So even with the change to XPCJSRuntime::SuspectWrappedNative (which will be done as part of bug 614347) we keep windows alive. It looks like we do need changes to GC scheduling. I've been able to reload about:home 50 times without any GC happening. We do run a bunch of CC's, but those aren't able to collect anything because anything held by the JS objects is not collectable. I think the current GC scheduling is having a really bad effect on the effectiveness of the CC.
Blocks: 624549
Assignee | ||
Comment 19•14 years ago
|
||
Peter, could you try attachment 509451 [details] [diff] [review]?
Comment 20•14 years ago
|
||
Reloading would be a perfect time for GC but it might slow town page transitions :(
Assignee | ||
Comment 21•14 years ago
|
||
We have had GC+CC after page load for ages. Bug 624549 made those less likely,
and attachment 509451 [details] [diff] [review] makes them again more likely.
Comment 22•14 years ago
|
||
From a quick test it looks like attachment 509451 [details] [diff] [review] helps here. Smaug, want to take the bug?
For the record the marking problem (comment 17) will be fixed with bug 614347.
Assignee | ||
Updated•14 years ago
|
Assignee: peterv → Olli.Pettay
Assignee | ||
Updated•14 years ago
|
Attachment #509451 -
Flags: review?(gal)
Updated•14 years ago
|
Attachment #509451 -
Flags: review?(gal) → review+
Updated•14 years ago
|
Hardware: x86 → All
Whiteboard: [hardblocker] → [hardblocker][has patch][needs landing]
Version: unspecified → Trunk
Comment 24•14 years ago
|
||
sayrer, can you turn this into a testcase? we should not have such massive leaks and none of our tests tell us
Assignee | ||
Comment 25•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/0a2e06927c31
but keeping this still open because of comment 18 (about bug 614347)
Whiteboard: [hardblocker][has patch][needs landing] → [hardblocker][has patch]
Comment 26•14 years ago
|
||
I'm not sure if this is related, but load about:memory and hold F5
Easy to monitor some mem numbers, steady grow until GC, then grow bit more again, goto GC
PS. Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110209 Firefox/4.0b12pre
Comment 27•14 years ago
|
||
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/d07aab3b3b7c
Note: not marking as fixed because fixed-in-tracemonkey is not present on the whiteboard.
Comment 28•14 years ago
|
||
I think we can mark this a dup of 630932 now. That bug will take care of GC and CC scheduling. That bug should land shortly. Please re-open if you disagree.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Updated•14 years ago
|
Blocks: mlk-fx4-beta
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•