David Baron :dbaron: Assignee
	Description • 23 years ago

I've been investigating why we occasionally leak nsXULPrototypeScript::mJSObject
and sometimes nsXULPrototypeAttribute::mEventHandler JS roots.  The problem is
that we call RemoveJSGCRoot more than we call AddJSGCRoot (these are in
nsXULElement.cpp).  When we do this, the gJSRuntime gets set to null early, and
we fail to remove the same number of roots as there were excess calls, except
the ones we don't remove are the ones called last.  This means we leak real roots.

I haven't figured out the cause of the imbalance yet, but I'm still looking...

	David Baron :dbaron: Assignee
	Comment 1 • 23 years ago

XULContentSinkImpl::OpenScript and nsXULDocument::LoadScript seem to be filling
in the mJSObject of an nsXULPrototypeScript without rooting it (presumably
because it's already in the XUL cache and thus rooted).

	David Baron :dbaron: Assignee
	Comment 2 • 23 years ago

Brendan caused this in the fastload landing.  I have one idea for how to fix it,
but it may not be the best idea.

	David Baron :dbaron: Assignee
	Comment 4 • 23 years ago

This fix shouldn't cause too much extra work, I think, since all
nsXULPrototypeScript eventually get an mJSObject.  Presumably that happens
pretty quickly, so there won't be many (if any) GC runs before they do (that
would cause an extra call to gc_root_marker that would break out since v is
null).  But maybe there's a better way.  (And I wonder if we have similar
problems for nsXULPrototypeAttribute::mEventHandler (maybe bug 87040?), where a
fix like this one *would* be a big performance cost.)

	Brendan Eich [:brendan]
	Comment 5 • 23 years ago

dbaron: I like your patch; the redundant roots don't cost much (e.g., for
strres.js when it's hit in the XUL script cache) and it all works well even in
the face of imbalance leading to too many Removes.  r=brendan@mozilla.org.  Hey
waterson, care to sr=?

/be

	Brendan Eich [:brendan]
	Comment 6 • 23 years ago

Thanks again to dbaron for tracking this down, patching, and offering to do the
checkin.  Cc'ing shaver for sr= cuz waterson's playing b-ball.

/be

	David Baron :dbaron: Assignee
	Comment 7 • 23 years ago

ok, taking

	Chris Waterson
	Comment 8 • 23 years ago

sr=waterson

	David Baron :dbaron: Assignee
	Comment 9 • 23 years ago

Fix checked in 2001-08-01 19:07 PDT.