bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

leak nsXULPDGlobalObject starting up and quitting Firefox on Mac (RLk nonzero)

RESOLVED FIXED in mozilla1.9alpha8



11 years ago
2 years ago


(Reporter: dbaron, Assigned: dbaron)



Mac OS X

Firefox Tracking Flags

(Not tracked)



(2 attachments)



11 years ago
Starting and shutting down firefox on Mac, we leak an nsXULPDGlobalObject.  So far I've found that it's due to unbalanced js_LockGCThing calls due to multiple calls to nsXULPrototypeCache::PutScript.  I see the following assertion fire once for each unbalanced js_LockGCThing (and it would cause an unbalanced js_LockGCThing due to the extra HoldScriptObject in that function):

NS_ASSERTION(!mScriptTable.Get(aURI, nsnull), "already have script in table");

Note that we're creating two scripts for the URI, and the first one put in the prototype cache is overwritten in the cache and thus leaked.

Comment 1

11 years ago
Er, I should have been clearer that I added that assertion at the start of nsXULPrototypeCache::PutScript.

Comment 2

11 years ago
Created attachment 277104 [details]
nsXULPrototypeCache::PutScript calls during startup

Comment 3

11 years ago
The problem seems to be that two different nsXULDocument objects are loading (and blocking on) the same sequence 5 scripts at the same time.  Not surprisingly, one of them is chrome://browser/content/hiddenWindow.xul and the other is chrome://browser/content/browser.xul .

Comment 4

11 years ago
It seems like what we should really be doing here is keeping a table of loading scripts in the XUL prototype cache, and adding additional listeners to existing loads.  (I think we'd then need to have one of the listeners compile the script and put it in the cache and the rest get a different notification.)

And we should add the assertion above...
Assignee: dbaron → nobody

Comment 5

11 years ago
Then again, maybe I should just do the simple leak fix now and worry about the rest later.
Assignee: nobody → dbaron
Note, by the way, that we have a similar issue with overlays.  They're placed in the cache much later than NewChannel(), so two NewChannel()s for the same overlay in quick succession will both try to start overlay loads, then in nsXULDocument::StartDocumentLoad we'll find the overlay in the cache and create a CachedChromeStreamListener which will assert in OnDataAvailable...

I don't think we clobber the cache in that case, though.

What I'd do here is do the simple leak fix, and a followup bug on adding the assert and making this code coalesce better.

Comment 7

11 years ago
Created attachment 277141 [details] [diff] [review]
simple leak fix

Here's the obvious leak fix; I filed bug 392650 on the underlying problem, which is a bit more work.
Attachment #277141 - Flags: superreview?(bzbarsky)
Attachment #277141 - Flags: review?(enndeakin)
Comment on attachment 277141 [details] [diff] [review]
simple leak fix

Looks reasonable.
Attachment #277141 - Flags: superreview?(bzbarsky) → superreview+


11 years ago
Attachment #277141 - Flags: review?(enndeakin) → review+


11 years ago
Attachment #277141 - Flags: approval1.9?
Attachment #277141 - Flags: approval1.9? → approval1.9+

Comment 9

11 years ago
Fix checked in to trunk.
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M8

Comment 10

11 years ago
Hrm.  On tinderbox, all this fixed was:

nsSystemPrincipal                                 0    -100.00%
nsXULPDGlobalObject                               0    -100.00%

whereas on my machine it had fixed a good bit more (although I was just starting up and shutting down; not running the page cycler).  So there's still more investigation needed for the leaks on tinderbox.


10 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets


2 years ago
Depends on: 1244948
You need to log in before you can comment on or make changes to this bug.