Iframes Memory Leak with jQuery and Sizzle

[vamsikrishnach90]

Description

We have a situation where there are 3 levels of nested iframes. The 3rd iframe initializes few heavy size objects, but this iframe is discarded and rebuilt in the DOM whenever user navigates across the application.

A memory leak is noticed where heap size of the application keeps on increasing whenever a new iframe at 3rd level is discarded and constructed.

Link to test case

Goto https://vamsikrishnach90.github.io/ in Chrome. Open developer tools (Hit F12) and navigate to Memory Tab. Pay attention to the JS heap size which is arppox 5 MB.

1. Click on 'Load Child Page' button. A child page is loaded inside the iframe of parent page.
2. Click on 'Bombard the page!' button on the child page. This will initialize 1 global variable of custom type with a huge json. Open devtools and check for the heap which is now 75 MB.
3. Click on 'Remove Child Page' button. The child page (iframe) is removed from the DOM.

Repeat steps 1 - 3 for about 2 more times. After step 2, note down the heap size. You'll notice that the heap keeps on increasing every time we add a child iframe and bombard it. Even though at step 3, we discard the frame, the heap is not claimed by the garbage collector.
Usecase demo help (Picture)

Observations

After repeating above 1-3 steps for 3x times, take a heap snapshot. Notice that the 3 instances of custom class 'Vamsi' that were created every time we added the child page and bombarded it still exist in the heap, even though the corresponding iframes are deleted from the DOM.

Notice in the object graph, that an unloadHandler() has been bound to the Window object belonging to the discarded iframes. Heap Memory Snapshot

Root Cause

The child page has a jquery lookup statement in the parent page:
$('div.msg-banner', window.parent.document).show();

This one line statement is causing the issue for memory leak.

$('div.msg-banner', window.parent.document)

The above part of the statement, calls JQuery find api. This find API is overridden by Sizzle, a pure-JavaScript CSS selector engine adopted by JQuery. This find api from JQuery calls Sizzle's setDocument function See Picture which registers an unloadHandler() with the requested document's(window.parent.document) window object. This is leading in the parent page's unload event bound to unloadHandler() callback function sitting in the child page. Hence, even though the child page iframe is removed from the DOM, the window object is never eligible to be garbage collected and is the reason for memory leak.

Link to Stackoverflow thread: https://stackoverflow.com/questions/76579826/iframes-memory-leak-with-jquery-and-sizzle

[mgol]

Thanks for the report. The unload handler is needed by IE & Edge Legacy; in jQuery 4 that drops support for the latter, we'll explicitly only set it in IE, avoiding the memory leak outside of IE.

To avoid the leak, IE or not, we'd probably have to add an additional unload handler on the document on which jQuery is loaded that would collect all the unload handlers attached to other documents and remove them when the main document is unloaded. This might require significant code to pull off, though, which I'm not too happy about.

[mgol]

We could also consider limiting the issues to IE & Edge Legacy even in the 3.x line by doing an indirect support test - by checking of presence of documentElement.msMatchesSelector: https://caniuse.com/matchesselector

Neither IE nor Edge Legacy are developed anymore and I cannot imagine modern browsers now suddenly adding this non-standard API so such a check should be safe.

A real fix for all browsers would probably require defining something like:

function setupUnloadHandler( subWindow ) {
	function unloadHandler() {
		window.removeEventListener( "unload", unloadHandler );
		subWindow.removeEventListener( "unload", unloadHandler );
		setDocument();
	}
	window.addEventListener( "unload", unloadHandler );
	subWindow.addEventListener( "unload", unloadHandler );
}

and then, in the place where we currently attach unloadHandler, to instead call:

setupUnloadHandler( subWindow );

That would be +24 bytes, though. If we went for this, I'd also want to first make sure this still fixes the IE issues and I have troubles actually reproducing it in IE 11 with the unloadHandler logic removed. I looked at test cases from https://bugs.jquery.com/ticket/13936 & https://bugs.jquery.com/ticket/14535 and none of them fail without the patch. I suspect the code got simplified and there are fewer code paths that don't already invoke setDocument early enough to hit the issue. But if we want to do this, I'd like to first find such a code path.

Help with finding a good test case is welcome!

[vamsikrishnach90]

Thanks for responding and acknowledging the problem. Let me know when you have next "Plan of Actions" to fix this issue after your internal meeting and review.

[mgol]

Just for reference - @vamsikrishnach90 would you be satisfied if we fix the issue outside of IE & Edge Legacy or do you care about those legacy browsers as well?

I’m asking because every workaround incurs cost: additional size, testing & maintenance burden. This issue existed in the code base for ages and AFAIK this is the first report we have about it. That suggests it may be an edge case and caring about it in IE might be an even bigger edge case.

[vamsikrishnach90]

I am fine with the fix being available to all other browsers except IE & Edge Legacy. @mgol Which version of JQuery do you think is going to receive a fix for this?

[mgol]

We still need to discuss this but I'd expect changes to arrive in jQuery 3.7.1.

[mgol]

PR: #5282

[mgol]

My workaround from #5281 (comment) would require more changes as setDocument() should only be called for subWindow unload so handlers need to be different. I think for now I prefer to just skip the workaround outside of IE & Edge Legacy and accept the memory leak in those two browsers.

[mgol]

Fixes for non-IE non-Edge Legacy browsers in #5282 - and that's all we plan, closing.