Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Jqlite data leak #7966

Closed

Conversation

petebacondarwin
Copy link
Contributor

This PR sits on top of #7942 so only the last commit is relevant here while the previous PR is not merged into master.

If an element contains two "element" transcludes then the initial clone
consists of only comment nodes.  The concern was that this meant that
the transclude scopes would not be cleaned up.

But it turns out that in the case that there are only comments then the
scope is never attached to anything so we don't need to worry about cleaning
it up.

Later if a concrete element is created as part of the transclude then these
elements will have destroy handlers.
We were attaching handlers to comment nodes when setting up bound transclusion
functions. But we don't clean up comments and text nodes when deallocating so
there was a memory leak.

Closes angular#7913
Calling `jqLite.data()` on a disallowed node type caused an empty object to be added to the
cache. This could lead to memory leaks since we no longer clean up such node types when they are
removed from the DOM.
@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#7966)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@petebacondarwin petebacondarwin added this to the 1.3.0-beta.14 milestone Jun 24, 2014
ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
Calling `jqLite.data()` on a disallowed node type caused an empty object to be added to the
cache. This could lead to memory leaks since we no longer clean up such node types when they are
removed from the DOM.

Closes angular#7966
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants