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 <rich.harris@vercel.com> * Update .changeset/great-crabs-rhyme.md Co-authored-by: Rich Harris <rich.harris@vercel.com> --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> Co-authored-by: Rich Harris <rich.harris@vercel.com>
sveltejs · Nov 26, 2024 · 45417a3 · 45417a3
1 parent a6ad5af
commit 45417a3
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 15 deletions.
diff --git a/.changeset/great-crabs-rhyme.md 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
@@ -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<V>} */
 	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
@@ -23,14 +23,15 @@ export interface Reaction extends Signal {
 	fn: null | Function;
 	/** Signals that this signal reads from */
 	deps: null | Value[];
-	parent: Effect | null;
 }
 
 export interface Derived<V = unknown> extends Value<V>, Reaction {
 	/** The derived function */
 	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
@@ -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
@@ -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);
+		};
+	});
 });