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...

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).

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

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.)

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

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

ok, taking

sr=waterson

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