Skip to main content

Bug Tracker

Side navigation

#14674 closed bug (migrated)

Opened January 07, 2014 03:56AM UTC

Closed October 21, 2014 12:31AM UTC

.off() does not clean up empty cache entries

Reported by: psquared Owned by:
Priority: low Milestone: 1.12/2.2
Component: data Version: 2.0.3
Keywords: Cc: rwaldron
Blocked by: Blocking:
Description

Even if all event handlers are removed with .off(), an empty cache entry is left in data_priv. Unfortunately, those empty cache entries remain if elements are removed using some method other than .remove(), causing a memory leak.

Test Environment: \\\\

Chrome 31.0.1650.63, jQuery 2.0.3

Steps to Reproduce:\\\\

1. Attach an event handler to an element (e.g. using .on())

2. Remove the event handler with .off()

3. Remove the element with Node.removeChild()

Expected Results:\\\\

No memory leaks.

Actual Results:\\\\

An orphaned cache entry remains in data_priv.

Example:\\\\

http://jsfiddle.net/Rx3v3/

Additional info:\\\\

Modifying the last block of jQuery.event.remove() as below seems to alleviate the problem.

#!js
// Remove the expando if it's no longer used
if ( jQuery.isEmptyObject( events ) ) {
    delete elemData.handle;
    data_priv.remove( elem, "events" );

    // Clean up empty cache entries:
    delete data_priv.cache[elem[data_priv.expando]];
}
Attachments (0)
Change History (6)

Changed January 07, 2014 04:10PM UTC by psquared comment:1

Perhaps this is a better suggestion than the one I made above--adding something like this to the end of Data.remove() seems to help:

if (jQuery.isEmptyObject(cache)) {
	this.discard(owner);
}

Changed January 07, 2014 04:21PM UTC by dmethvin comment:2

cc: → rwaldron
version: 1.10.22.0.3

@rwaldron could you take a look?

Changed January 07, 2014 04:36PM UTC by gibson042 comment:3

In general, we do not support mixing native DOM manipulation with jQuery. This is close to $( el ).data({ leaks: true })[0].parentNode.removeChild( el ), but distinct enough that it may be worth looking into a small fix.

Changed January 07, 2014 04:48PM UTC by dmethvin comment:4

I agree with that, there are plenty of ways you can get into trouble if you use native methods to remove elements behind jQuery's back.

But in the 1.x branch if you remove all events and there is no other data in the object, it removes the data object preemptively. e.g.,

https://github.com/jquery/jquery/blob/8b88ca28874cd990d25854a6fea0565b716bb482/src/event.js#L223

https://github.com/jquery/jquery/blob/8b88ca28874cd990d25854a6fea0565b716bb482/src/data.js#L205

Changed January 20, 2014 11:35PM UTC by dmethvin comment:5

component: unfileddata
milestone: None1.12/2.2
priority: undecidedlow
status: newopen

I'll mark this valid pending investigation. Since events are often the only thing in the data cache it would be helpful to remove them when all events are gone.

Changed October 21, 2014 12:31AM UTC by m_gol comment:6

resolution: → migrated
status: openclosed