Opened 9 years ago
Closed 9 years ago
#14674 closed bug (migrated)
.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: | Rick Waldron | |
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:
- Attach an event handler to an element (e.g. using
.on()
) - Remove the event handler with
.off()
- 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.
// 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]]; }
Change History (6)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Cc: | Rick Waldron added |
---|---|
Version: | 1.10.2 → 2.0.3 |
@rwaldron could you take a look?
comment:3 Changed 9 years ago by
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.
comment:4 Changed 9 years ago by
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
comment:5 Changed 9 years ago by
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.
comment:6 Changed 9 years ago by
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: