Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Component anchor memory leak #2458

Closed
basvanmeurs opened this issue Oct 22, 2020 · 0 comments · Fixed by #2459
Closed

Component anchor memory leak #2458

basvanmeurs opened this issue Oct 22, 2020 · 0 comments · Fixed by #2459
Labels
has PR A pull request has already been submitted to solve the issue

Comments

@basvanmeurs
Copy link
Contributor

basvanmeurs commented Oct 22, 2020

Version

3.0.2

Reproduction link

https://codepen.io/basvanmeurs/full/dyXpPJY

Steps to reproduce

  1. Open link
  2. Wait for 2s until only one 'my-test' heading remains
  3. Make heap snapshot

What is expected?

There should be 1 instances of IAmStillThere

What is actually happening?

There are 2 instances of IAmStillThere rather than 1


The removed item's instance and full scope (IAmStillThere) is still in memory, because it is retained by the instance.

Open the memory profiler and create a snapshot. Search for IAmStillThere. Observe that there are 2 rather than 1. Observe that the top one (which belongs to key "2") should not be there as it was removed.

The top IAmStillThere is retained because the handler was attached to the vnode as a prop. This vnode is retained because it was the anchor which was passed over to setupRenderEffect for the initial patch only of the component with key "1".

Later on, as "2" is removed, the anchor will remain in the scope of instance.update of "1". So the HTMLElement will leak, and become detached (search for 'Detached HTMLHeadingElement' in the snapshot).

The background of the problem is that setupRenderEffect servers two separate purposes: Mounting and updating. A big if-else statement distinguishes between the two. During mounting, anchor is used to invoke the patch. Anchor is never used while updating (and better so, as it is only valid within the current patch tree state). Even though it will never be used again, it will still stay in the scope of instance.update, which causes it to cling on to things that it shouldn't cling on to.

In my own use case, an advanced scroller, when scrolling up this bug caused all previously visible rows to stay in memory, building up to a major leak.

The solution for this specific case is actually quite simple: dereference immediately after mounting by setting to null.

Let's also do this for initialVNode and container. I don't have an example of how this can go wrong, but I suspect that it can go wrong in edge cases (teleports?). In any way, it's never a bad idea to clean up after yourself ASAP ;-)

@posva posva added the has PR A pull request has already been submitted to solve the issue label Nov 18, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Oct 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has PR A pull request has already been submitted to solve the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants