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:\\\\
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 comment:1
Changed January 07, 2014 04:21PM UTC by comment:2
cc: | → rwaldron |
---|---|
version: | 1.10.2 → 2.0.3 |
@rwaldron could you take a look?
Changed January 07, 2014 04:36PM UTC by 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 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 comment:5
component: | unfiled → data |
---|---|
milestone: | None → 1.12/2.2 |
priority: | undecided → low |
status: | new → open |
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 comment:6
resolution: | → migrated |
---|---|
status: | open → closed |
Migrated to https://github.com/jquery/jquery/issues/1760
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: