"on" events leak memory in the checkContext variable

Reported by:	giulio.dali@mdsl.com	Owned by:	giulio.dali@mdsl.com
Priority:	undecided	Milestone:	None
Component:	unfiled	Version:	1.9.1
Keywords:		Cc:	gibson042, timmywil
Blocked by:		Blocking:

Description

I've created a small sample which shows how the "on" function can leak memory. This was tested in Chrome 26, looking at the Memory Heap. To reproduce:

1) Load this test page (I've not used jsFiddle as it has it's own leaks and confuses the situation).

<html>
<head>
    <script src="//ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.js"></script>
</head>
<body>

<div class="i_will_leak">
    <input type="text" />
</div>
<button>Leak</button>

<script>
    $(function () {

        $('div').on('click', 'input', function () {
            this.value = 'clicked';
        })
        
        $('button').click(function () {
            $('div').off().unbind().remove();
        })
    });
</script>
</body>
</html>

2) Click on the input box

3) Click the "Leak" button. This simply removes all events and trashes the element. (I've purposefully called all the removes I can think of, although remove() should suffice)

4) Check the memory profile for detached nodes and the "i_will_leak" DIV will have leaked against the checkContext variable.

NOTE: This ONLY happens if you actually click the input box, if you don't interact with it, then it doesn't leak. Hence the issue is in dispatching the event, rather than attaching or removing it.

Expected Result: It shouldn't leak

I hope this is enough information.

Attachments (0)

Change History (7)

Changed May 17, 2013 02:47PM UTC by dmethvin comment:1

owner:	→ giulio.dali@mdsl.com
status:	new → pending

It's just caching the selector there I think, we don't attempt to removed cached selectors because they can be reused. Or are you saying this continues without bound?

Changed May 17, 2013 02:51PM UTC by giulio.dali@mdsl.com comment:2

status:	pending → new

I don't believe it is as I've not seen it go away. Unless you have examples of how a subsequent query clears the cache? Surely if I've removed the event and node there is noting left to cache?

Changed May 17, 2013 02:56PM UTC by dmethvin comment:3

status:	new → pending

It won't go away, but it also shouldn't happen a second time. You've cached a selector there.

My question, restated, is: Does it "leak" every time you run that, or only the first time? You'll need to redo your example to allow you to re-attach the event handler.

Changed May 17, 2013 03:09PM UTC by giulio.dali@mdsl.com comment:4

status:	pending → new

Hi,

I'm not sure I follow 100%, but I've changed the sample to allow a recreation and does appear to leak only once. Please confirm this is correct:

<html>
<head>
    <script src="//ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.js"></script>
</head>
<body>

<div id="newAdd">
</div>

<button id="leak">Leak</button>


<button id="readd">Re-add</button>

<script>

    function buildSample() {
        $('#newAdd').append('<div class="i_will_leak"><input type="text" /></div>');
        $('.i_will_leak').on('click', 'input', function () {
            this.value = 'clicked';
        })
    }

    $(function () {

        $('#leak').click(function () {
            $('.i_will_leak').off().unbind().remove();
        })

        $('#readd').click(buildSample)

        buildSample();

    });
</script>
</body>
</html>

Thanks.

Changed May 17, 2013 03:36PM UTC by dmethvin comment:5

cc:	→ gibson042, timmywil

We're caching a reference to the selector the first time we see it, so that we don't need to recompute it each time it's used. This is not a leak, we still have and in the future will use the reference to the code used to compute the 'input' selector string when needed.

timmywil, gibson042 am I misinterpreting this?

Changed May 17, 2013 04:05PM UTC by gibson042 comment:6

You've basically got it, dmethvin... as documented in the source:

The foundational matcher ensures that elements are reachable from top-level context(s)

We store a reference to the context and then test it with one of two functions (based on whether context is a node or an array). This allows us to use the same code when searching subtrees as we do with entire documents.

Changed May 17, 2013 04:10PM UTC by dmethvin comment:7

resolution:	→ notabug
status:	new → closed

Okay, that's what I thought.

Side navigation

#13904 closed bug (notabug)