Bug 17523 – Javascript creating a protected object for onclick and not GCing once no-longer shown

• Component: WebCore JavaScript
• Status: RESOLVED INVALID

[Shawn Stricker (kb1ibt)]

On the page there are new protected objects being created for every element assigned with a onclick function. Specifically looking at the runner.js file inside the createLI function  there is an object being assigned with a function via onclick which seems to cause the object to become protected. Since that page updates every 5 seconds after a couple hours safari will report an out of memory issue in the javascript console.

[Shawn Stricker (kb1ibt)]

Created attachment 19326 [details]
reduction of error

This file is a reduction of the bug, it refreshes every 5 seconds and every refresh creates a new protected object.

[Mark Rowe (bdash)]

With TOT the "Out of Memory" error will no longer occur, but the increase in the number of protected objects is a bit suspicious.

[Mark Rowe (bdash)]

<rdar://problem/5762351>

[Geoffrey Garen]

I don't really see how we can avoid this memory use. It would be an error to strip the element of its event handlers just because it was removed from the document, since the element can be re-added to the document.

[Geoffrey Garen]

Sorry -- I realize now that the node is no longer reachable after it's removed from the document, so, in theory, it should be GC'd along with its event handler.

[Geoffrey Garen]

Turns out this bug is invalid.

The reduction says:

var existingDiv = document.getElementById('test');
...
newDiv.onclick = function(){alert("You clicked me")};


The function expression assigned to the "onclick" handler implicitly captures "existingDiv" in its scope chain / closure. So, each new div inserted into the document references an event handler that references the last div inserted into the document. It's a giant linked list, implicitly created by closures.

In theory, an optimizing compiler could prove that the event handler did not actually reference "existingDiv", and optimize it out of the closure. But that's probably beyond the scope of what this bug intended.

Work around: Remove "existingDiv" from the scope in which the event handler is defined.