386294 - leak nsJSRuntime due to unbalanced HoldScriptObject(nsnull)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We leak an nsJSRuntime on Firefox startups when we're reading from fastload because we leak references of the sHoldScriptRootCount in nsContentUtils.  This is because one of the nsXULPrototypeScript deserialization codepaths calls mScriptObject.set(nsnull), which calls HoldScriptObject(nsnull), which works since js_LockGCThingRT silently no-ops on null input, and then checks mScriptObject.mObject to see if it has already called HoldScriptObject, and then does it again with a good pointer.  This leads to sHoldScriptRootCount ending up off.

There's a question of how far we should allow the null.  We could allow it in nsContentUtils::ScriptObjectHolder, but I think we have to detect it in or before nsContentUtils::HoldScriptObject/DropScriptObject.  However, my inclination is to forbid it from both and make the caller not pass null.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk.