Subscribe on changes!

Potential memory leak in computed

avatar
Sep 16th 2023

Vue version

3.3.4 (all 3.* versions I tested this with were affected)

Link to minimal reproduction

https://play.vuejs.org/#eNqdVMFu2zAM/RXBpxRI7EN3ypKg29AB7aEd2l0G+KLYdKJWlgyJTpoF/vdRkp0oSTsM66Wx+PT4+ERyn3xpmnTTQjJNZrYwokFmAdtmkStRN9og27NC102LUI6ZgWrMthyL9W1VQYGsY5XRNcsTosiTXOWq0MoSh67hcflCkG8SuIGSzd3lUcWlhasBVVBMtQ3FFGzZd6G4FL85Cq2eYCUsmt3oFXZsvmB7okZRsfA9n1PGY4o8uQqAHB2vlpBKvRrFEJ+KZBA0AC8EphsuWyApaKgUwnS56hxYSYjreYKqr0W1Uh5Kifh6twg1GDcaXbkiTkhCPp8gMPCiAGvpVgDHldw/Pz6kZIdQK1HtRpe5erKIzdd7IPPuvJPeOU9VuOhHzrl6t9yeGEjWDHmEEvh8tPkkYZbFvdKbMET+u0C63F19HiTHzlP2PUN4wym1xxqk1GyrjSTNzCn+wICojxxnaMnU+AYEE2kZnzedN+LUgBEdzrIwSDRC9IFQN5Ij0NfMTVWOdxXb6ZZVwpD6xrhHz5Pw+qSUq5LhGhSdecvzZByXKCzNygbM8B6pY/xFdGu+AYa6J+ybia+4UO40tIPAdLY0TknmpPR6ZssWUSt2U0hRvM6PWhbhxywLgPfBvciF/38C3e/pOS4XQce6jrGDhuxgUDJO0NKbVmKVvlitaCVRsxA0cXMkJJjHxu0GUjZ1bcToj8TSM2/v/Zmb3PFwXqyheH3n/MW+ubM8+UFGgdnQ2jrEkJsV0Mu68O3zA3VSFKx12UpC/yX4BNTRrdMYYF9bVZLsCOfV3vnFSr3+096+ISg7FBVWD2Odx/ul6gbgo9KPcq/Ta3+POpJcDIt7UvPmzMcQOCXxi9sPDGJjp1lWlIqulSDFxqQKMFNNnd0QLDOtQlHDpNT1DWVMP2UlzUh8nIKtJ0ujt+QskUSFu/vebjMxoEowzpZ/S3t2LU59FrpIfzCl+wNg0WvC

Steps to reproduce

Click on "access" and then on "clear". Even if you then manually trigger garbage collection in the browser, "someObject" doesn't get removed from memory. You have to click "access" again for this to happen.

What is expected?

That after "clear" is called "someObject" gets garbage collected because there shouldn't be anything referencing it.

What is actually happening?

"someObject" stays in memory until the computed property is accessed again.

System Info

No response

Any additional comments?

This was the cause of a huge memory leak in my application that took me very long to discover.

avatar
Sep 23rd 2023

This is not a memory leak. computed properties are cached, so when you modify the variables that a computed property depends on, it doesn't immediately change the computed property's value. In fact, it continues to cache the value from the last time you accessed it until the next get call, at which point it gets recalculated and cached again. During this recalculation, it releases the previously cached value.

https://github.com/vuejs/core/blob/b8fc18c0b23be9a77b05dc41ed452a87a0becf82/packages/reactivity/src/computed.ts#L59

avatar
Sep 25th 2023

like @Alfred-Skyblue said above. this is not a memory leak. as far as I know, vue-devtools will lead to memory leaks. @matej-svejda Could you test in your application with vue-devtools disabled?

avatar
Oct 2nd 2023

@Alfred-Skyblue I see, I guess that makes sense from a technical perspective. I'm not sure it's great from an API usage point of view, but I also don't see how to easily improve that.

@edison1105 I had vue-devtools turned off.

avatar
Oct 20th 2023

@johnsoncodehk If we want to solve this problem, we must run the effect every time set is executed, even if get is not triggered. Perhaps we should add a parameter to it to indicate whether it needs lazy loading, like lazy: false.

avatar
Oct 20th 2023

@Alfred-Skyblue I don't think the author's original problem needs fixing, it's expected behavior, but my playground link shows a different problem, someObject still doesn't get recycled even though computed itself is set to undefined.

avatar
Oct 26th 2023

I dug a bit more and it seems the case @johnsoncodehk discovered is an edge case.

  • It's true that simply setting the computed to undefined doesn't cause its cached value to be garbage collected.

  • However, if we put the same test case inside a child component and unmount that child component, the computed value is cleanly garbage collected. See this case: https://gist.github.com/yyx990803/5ed0bdcb3ff74dee34de1326acf143e7

    • I wasn't able to cleanly reproduce this in the SFC playground, as there might be other variables at play. A simple HTML file also allows us to more easily capture and analyze memory snapshots.
    • click access first, then click toggle, then force GC, you will see the object is collected. You can also use Memory snapshots to check for the presence of HeavyObject.
    • Note this is using the current released 3.3.7. I've verified it's the same behavior using this PR, but with WeakRef and FinalizationRegistry removed.

I'd argue the 2nd case is far more common (and probably 99.9% of the case) in real world apps. I've never seen anyone manually setting computed to undefined - they are always declared and used as a constant inside components, and expected to be garbage collected when the component is unmounted. If that works as expected, then I don't think we need to resort to FinalizationRegistry for the unlikely usage of computed.

avatar
Oct 26th 2023

Closing as the original issue is not considered a leak: a leak means the memory has a chance of infinitely growing over time - in this case, there will always only be at most 1 copy of a cached value held, and it is properly released on next access / component unmount.