Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

jQuery/Sizzle checkContext memory leak

While debugging my app with 'Profiles' in DevTools I found "Detached DOM tree's" accumulating. These detached nodes have retaining tree consisting mostly of checkContext functions (coming from sizzle inside jQuery - v1.10.1).

Heap snapshot

I'm not sure how to proceed with this. What does this result mean?

like image 793
Konrad Dzwinel Avatar asked Jun 24 '13 15:06

Konrad Dzwinel


2 Answers

Sizzle stores compiled selectors in selector cache, which by default stores up to 50 entries. You may experiment by setting $.expr.cacheLength = 1 before doing any selections and see if they go away.

Here's the docs https://github.com/jquery/sizzle/wiki/Sizzle-Documentation#-internal-api. Seems internal so don't depend on it or anything in actual production code.

like image 138
Esailija Avatar answered Oct 25 '22 09:10

Esailija


This is actually a bug, there's no reason that Sizzle needs to hang onto the context node, it's only doing it because it's not cleaning up after setting a temporary variable. I filed an issue for it, fixed it, ran all the Sizzle tests, and did a pull request.

If you want to patch your existing copy of jQuery or Sizzle:

  1. Open your jQuery or Sizzle file

  2. Search for the matcherFromTokens function

  3. Find this code within it (near the top):

    matchers = [ function( elem, context, xml ) {
        return ( !leadingRelative && ( xml || context !== outermostContext ) ) || (
            (checkContext = context).nodeType ?
                matchContext( elem, context, xml ) :
                matchAnyContext( elem, context, xml ) );
    } ];
    
  4. Change the return to var rv =, and add checkContext = undefined; and then return rv; at the end of the anonymous function, e.g.:

    matchers = [ function( elem, context, xml ) {
        var ret = ( !leadingRelative && ( xml || context !== outermostContext ) ) || (
            (checkContext = context).nodeType ?
                matchContext( elem, context, xml ) :
                matchAnyContext( elem, context, xml ) );
        // Release the context node (issue #299)
        checkContext = null;
        return ret;
    } ];
    

Note: That code assigns null to checkContext because apparently that's their style. If it were me, I'd've assigned undefined instead.

If there's any issue with the fix raised during the pull request / merge process, I'll update the answer.

It's better to keep letting Sizzle cache selectors, because jQuery uses the compiled selector stuff with event delegation, and you don't really want it to have to reparse and rebuild the matcher functions each time a relevant event occurs so it can figure out whether elements match it.


This isn't the only place jQuery holds onto elements in compiled selectors, unfortunately. Each place it does is probably a bug that could use fixing. I've only had the time to track down one other, which I've also reported and fixed (pending the pull request being landed):

If you search for "Potentially complex pseudos" you'll find this for the :not pseudo-selector:

pseudos: {
    // Potentially complex pseudos
    "not": markFunction(function( selector ) {
        // Trim the selector passed to compile
        // to avoid treating leading and trailing
        // spaces as combinators
        var input = [],
            results = [],
            matcher = compile( selector.replace( rtrim, "$1" ) );

        return matcher[ expando ] ?
            markFunction(function( seed, matches, context, xml ) {
                var elem,
                    unmatched = matcher( seed, null, xml, [] ),
                    i = seed.length;

                // Match elements unmatched by `matcher`
                while ( i-- ) {
                    if ( (elem = unmatched[i]) ) {
                        seed[i] = !(matches[i] = elem);
                    }
                }
            }) :
            function( elem, context, xml ) {
                input[0] = elem;
                matcher( input, null, xml, results );
                return !results.pop();
            };
    }),

The problem is in the function after the : in the conditional operator:

function( elem, context, xml ) {
    input[0] = elem;
    matcher( input, null, xml, results );
    return !results.pop();
};

Note that it never clears input[0]. Here's the fix:

function( elem, context, xml ) {
    input[0] = elem;
    matcher( input, null, xml, results );
    // Don't keep the element (issue #299)
    input[0] = null;
    return !results.pop();
};

That's all I have the time to track down at present.

like image 38
T.J. Crowder Avatar answered Oct 25 '22 09:10

T.J. Crowder