From e7a90c38f3935bc2bab37352b11ef32d6b746224 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 26 Nov 2024 12:32:18 +0000 Subject: [PATCH] fix: addresses memory leak when creating deriveds inside untrack --- .../internal/client/reactivity/deriveds.js | 42 +++++++++++++------ .../src/internal/client/reactivity/types.d.ts | 5 ++- .../svelte/src/internal/client/runtime.js | 13 ++++-- packages/svelte/tests/signals/test.ts | 26 ++++++++++++ 4 files changed, 69 insertions(+), 17 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 98fbfb0f52..50669d1701 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} + */ +export function get_derived_parent_effect(derived) { + var parent = derived.parent; + if (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..9810c18567 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -29,7 +29,12 @@ import { import { flush_tasks } from './dom/task.js'; import { add_owner } from './dev/ownership.js'; import { mutate, set, source } from './reactivity/sources.js'; -import { destroy_derived, execute_derived, update_derived } from './reactivity/deriveds.js'; +import { + destroy_derived, + execute_derived, + get_derived_parent_effect, + update_derived +} from './reactivity/deriveds.js'; import * as e from './errors.js'; import { lifecycle_outside_component } from '../shared/errors.js'; import { FILENAME } from '../../constants.js'; @@ -766,10 +771,10 @@ export function get(signal) { } } else if (is_derived && /** @type {Derived} */ (signal).deps === null) { var derived = /** @type {Derived} */ (signal); - var parent = derived.parent; + var parent_effect = get_derived_parent_effect(derived); - if (parent !== null && !parent.deriveds?.includes(derived)) { - (parent.deriveds ??= []).push(derived); + if (parent_effect !== null && !parent_effect.deriveds?.includes(derived)) { + (parent_effect.deriveds ??= []).push(derived); } } diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index e2d27267a3..2df3ca9e5d 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.only('nested deriveds clean up the releationships 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); + }; + }); });