From 45417a36ce8100095f91907cdb9d47bbeafbaff7 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 26 Nov 2024 19:18:35 +0000 Subject: [PATCH] fix: addresses memory leak when creating deriveds inside untrack (#14443) * fix: addresses memory leak when creating deriveds inside untrack * fix: addresses memory leak when creating deriveds inside untrack * changeset * Update packages/svelte/src/internal/client/reactivity/deriveds.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * fix * fix * fix * comment * Update packages/svelte/tests/signals/test.ts * Update packages/svelte/src/internal/client/reactivity/deriveds.js Co-authored-by: Rich Harris * Update .changeset/great-crabs-rhyme.md Co-authored-by: Rich Harris --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> Co-authored-by: Rich Harris --- .changeset/great-crabs-rhyme.md | 5 +++ .../internal/client/reactivity/deriveds.js | 42 +++++++++++++------ .../src/internal/client/reactivity/types.d.ts | 5 ++- .../svelte/src/internal/client/runtime.js | 19 ++++++++- packages/svelte/tests/signals/test.ts | 26 ++++++++++++ 5 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 .changeset/great-crabs-rhyme.md diff --git a/.changeset/great-crabs-rhyme.md b/.changeset/great-crabs-rhyme.md new file mode 100644 index 0000000000..b02f7bc9b2 --- /dev/null +++ b/.changeset/great-crabs-rhyme.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: prevent memory leak when creating deriveds inside untrack diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 98fbfb0f52..a679d307c4 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -42,6 +42,11 @@ export function derived(fn) { active_effect.f |= EFFECT_HAS_DERIVED; } + var parent_derived = + active_reaction !== null && (active_reaction.f & DERIVED) !== 0 + ? /** @type {Derived} */ (active_reaction) + : null; + /** @type {Derived} */ const signal = { children: null, @@ -53,12 +58,11 @@ export function derived(fn) { reactions: null, v: /** @type {V} */ (null), version: 0, - parent: active_effect + parent: parent_derived ?? active_effect }; - if (active_reaction !== null && (active_reaction.f & DERIVED) !== 0) { - var derived = /** @type {Derived} */ (active_reaction); - (derived.children ??= []).push(signal); + if (parent_derived !== null) { + (parent_derived.children ??= []).push(signal); } return signal; @@ -104,6 +108,21 @@ function destroy_derived_children(derived) { */ let stack = []; +/** + * @param {Derived} derived + * @returns {Effect | null} + */ +function get_derived_parent_effect(derived) { + var parent = derived.parent; + while (parent !== null) { + if ((parent.f & DERIVED) === 0) { + return /** @type {Effect} */ (parent); + } + parent = parent.parent; + } + return null; +} + /** * @template T * @param {Derived} derived @@ -113,7 +132,7 @@ export function execute_derived(derived) { var value; var prev_active_effect = active_effect; - set_active_effect(derived.parent); + set_active_effect(get_derived_parent_effect(derived)); if (DEV) { let prev_inspect_effects = inspect_effects; @@ -162,14 +181,13 @@ export function update_derived(derived) { } /** - * @param {Derived} signal + * @param {Derived} derived * @returns {void} */ -export function destroy_derived(signal) { - destroy_derived_children(signal); - remove_reactions(signal, 0); - set_signal_status(signal, DESTROYED); +export function destroy_derived(derived) { + destroy_derived_children(derived); + remove_reactions(derived, 0); + set_signal_status(derived, DESTROYED); - // TODO we need to ensure we remove the derived from any parent derives - signal.v = signal.children = signal.deps = signal.ctx = signal.reactions = null; + derived.v = derived.children = derived.deps = derived.ctx = derived.reactions = null; } diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 2cef49eb2d..395070fedd 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -23,7 +23,6 @@ export interface Reaction extends Signal { fn: null | Function; /** Signals that this signal reads from */ deps: null | Value[]; - parent: Effect | null; } export interface Derived extends Value, Reaction { @@ -31,6 +30,8 @@ export interface Derived extends Value, Reaction { fn: () => V; /** Reactions created inside this signal */ children: null | Reaction[]; + /** Parent effect or derived */ + parent: Effect | Derived | null; } export interface Effect extends Reaction { @@ -58,6 +59,8 @@ export interface Effect extends Reaction { first: null | Effect; /** Last child effect created inside this signal */ last: null | Effect; + /** Parent effect */ + parent: Effect | null; /** Dev only */ component_function?: any; } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 5b5b488824..8931d02009 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -767,9 +767,24 @@ export function get(signal) { } else if (is_derived && /** @type {Derived} */ (signal).deps === null) { var derived = /** @type {Derived} */ (signal); var parent = derived.parent; + var target = derived; - if (parent !== null && !parent.deriveds?.includes(derived)) { - (parent.deriveds ??= []).push(derived); + while (parent !== null) { + // Attach the derived to the nearest parent effect, if there are deriveds + // in between then we also need to attach them too + if ((parent.f & DERIVED) !== 0) { + var parent_derived = /** @type {Derived} */ (parent); + + target = parent_derived; + parent = parent_derived.parent; + } else { + var parent_effect = /** @type {Effect} */ (parent); + + if (!parent_effect.deriveds?.includes(target)) { + (parent_effect.deriveds ??= []).push(target); + } + break; + } } } diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index e2d27267a3..db061bfb55 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -739,4 +739,30 @@ describe('signals', () => { assert.deepEqual(a.reactions, null); }; }); + + test('nested deriveds clean up the relationships when used with untrack', () => { + return () => { + let a = render_effect(() => {}); + + const destroy = effect_root(() => { + a = render_effect(() => { + $.untrack(() => { + const b = derived(() => { + const c = derived(() => {}); + $.untrack(() => { + $.get(c); + }); + }); + $.get(b); + }); + }); + }); + + assert.deepEqual(a.deriveds?.length, 1); + + destroy(); + + assert.deepEqual(a.deriveds, null); + }; + }); });