Closed Bug 412774 Opened 12 years ago Closed 12 years ago

Firefox leaks 10 nsGlobalWindows with Page Load Error Page


(Core :: General, defect, P2)






(Reporter: cbook, Assigned: peterv)



(Keywords: memory-leak)


(4 files, 2 obsolete files)

Attached file leak log
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008011704 Minefield/3.0b3pre

Steps to reproduce:
Create a new Profile to make sure you do not run into an second leak
(in my case i found the bug after i imported IE data at the migration wizard).
Close Firefox, to make sure its not a follow up leak from the migration

Firefox starts with 2 Tabs (both the default ones - firstrun and default startpage) 
In the first tab go to a Page Load Error page. As example (i wanted to test the new Sumo Project and did a typo instead the correct .com)
Wait till you get till you get the Page Load Error Page
Close Firefox

 0 TOTAL 28  1056402   332664    37213 ( 1169.30 +/-  1268.80)  1065981   

Same Problem happen after restart when both tabs (the Page Load Error) and the default Homepage is restored

 => mAllocCount:          19337
 => mReallocCount:         3070
 => mFreeCount:           15656  --  LEAKED 3681 !!!
 => mShareCount:          17258
 => mAdoptCount:           1692
 => mAdoptFreeCount:       1675  --  LEAKED 17 !!!

Note: during shutdown i got a lot of :
###!!! ASSERTION: XPConnect is being called on a scope without a 'Components' property!

This is pretty much always bad. It usually means that native code is
making a callback to an interface implemented in JavaScript, but the
document where the JS object was created has already been cleared and the
global properties of that document's window are *gone*. Generally this
indicates a problem that should be addressed in the design and use of the
callback code.
: 'Error', file c:/debug/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp, line 702

This was reproducible on Windows XP Debug Build and also on Vista Debug Build. I think this is a bad leak, since typos during surfing are a common thing...
Flags: blocking1.9?
Attached file new leak log
new test after peters patch was checked in :

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012014 Firefox/3.0b3pre

nsTraceRefcntImpl::DumpStatistics: 726 entries
nsDOMNodeAllocator leaked 128096 bytes 
 => mAllocCount:          20094
 => mReallocCount:         3049
 => mFreeCount:           16792  --  LEAKED 3302 !!!
 => mShareCount:          15834
 => mAdoptCount:           1272
 => mAdoptFreeCount:       1269  --  LEAKED 3 !!!
so far no luck with reproducing on linux.
Attached file linux leak log
(In reply to comment #2)
> so far no luck with reproducing on linux.

hmm, i'm still able to reproduce this, also on linux with Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9b3pre) Gecko/2008012104 Minefield/3.0b3pre

can you reproduce this when you do the following steps:

-> Create a new profile (or rename the firefox profile folders)
-> Start Firefox
-> Firefox comes up with the firstrun tab and startpage
change the url of the firstrun page to a page that cause the Page Load Error Site (like
-> When you see the error page, close Firefox and click if the tabs should be saved for the next start
-> wait till the 2nd tab with the start page is loaded and close firefox

Nope, still no leak, with new profile and all.
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9b3pre) Gecko/2008012113 
I have a hard time reproducing this too, but on OS X.
Anyone else able to reproduce?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
I'm going to leave this as a nom until we can confirm.
Flags: blocking1.9+ → blocking1.9?
Tomcat can reproduce on all platforms so confirming and upping priority.
Flags: blocking1.9? → blocking1.9+
Priority: P3 → P2
Great.  Thanks guys.
Peter, tomcat says he'll show you this one during the work week.
Assignee: nobody → peterv
Blocks: 414514
Blocks: 416607
Attached patch v1 (obsolete) — Splinter Review
Still looking for a better way to fix this. It looks like we keep tabs alive late into shutdown from session restore when we never got an onload notification for them. This happens for error pages and for pages that we've interrupted loading (for example by quitting while the page is loading).
Duplicate of this bug: 416607
Blocks: 417648
(In reply to comment #11)
> Created an attachment (id=302868) [details]

Should you want to go down that way, please (1) move this bit into sss_onClose, (2) add a comment as to why you're doing it, (3) delete the _tab property from |tabState| in sss_onTabClose as well and (4) ask me for review.
(In reply to comment #13)
> (1) move this bit into sss_onClose,
... either to the very end, or set _tab to a new object ({}) so as to not regress bug 415484.

> (4) ask me for review.
... provided, of course, that this fixes the actual issue.
Attached patch v2 (obsolete) — Splinter Review
Yes, this works too. And I've finally managed to track down the difference between regular page loads and error pages, onTabLoad is never called for error pages.
Attachment #302868 - Attachment is obsolete: true
Attachment #304718 - Flags: review?(zeniko)
Comment on attachment 304718 [details] [diff] [review]

>+      aWindow.__SS_dyingCache.tabs[t]._tab = {};

Please only do this, when there's actually a _tab we were referencing.

>+    tabState._tab = {};

This one won't be needed any more, so you could also just |delete| it (one less object to carry around).

r=me with these two nits fixed.

Thanks for catching this bug, Peter!
Attachment #304718 - Flags: review?(zeniko) → review+
(In reply to comment #16)
> (From update of attachment 304718 [details] [diff] [review])
> >+      aWindow.__SS_dyingCache.tabs[t]._tab = {};
> Please only do this, when there's actually a _tab we were referencing.

What would be the preferred way to do this?
(In reply to comment #17)
  if (aWindow.__SS_dyingCache.tabs[t]._tab) ...

OTOH, since you've put the code at the end of sss_onClose you can also just |delete| the _tab property (it's last needed for the _collectWindowData call above as an indicator whether the tab's been completely loaded: if it hasn't, we don't collect form and scroll data for it).
Attached patch v2.1Splinter Review
This is what I'll check in.
Attachment #304718 - Attachment is obsolete: true
Attachment #305353 - Flags: review+
OS: Windows Vista → All
Hardware: PC → All
Target Milestone: --- → mozilla1.9beta4
Tomcat: could you check whether any of the bugs that this one blocks are fixed too?
Closed: 12 years ago
Resolution: --- → FIXED
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008022515 Firefox/3.0b4pre , thanks peter !
You need to log in before you can comment on or make changes to this bug.