Skip to content

Commit 45417a3

Browse files
trueadmdummdidummRich-Harris
authored
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 <[email protected]> * 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 <[email protected]> * Update .changeset/great-crabs-rhyme.md Co-authored-by: Rich Harris <[email protected]> --------- Co-authored-by: Simon H <[email protected]> Co-authored-by: Rich Harris <[email protected]>
1 parent a6ad5af commit 45417a3

File tree

5 files changed

+82
-15
lines changed

5 files changed

+82
-15
lines changed

.changeset/great-crabs-rhyme.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: prevent memory leak when creating deriveds inside untrack

packages/svelte/src/internal/client/reactivity/deriveds.js

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ export function derived(fn) {
4242
active_effect.f |= EFFECT_HAS_DERIVED;
4343
}
4444

45+
var parent_derived =
46+
active_reaction !== null && (active_reaction.f & DERIVED) !== 0
47+
? /** @type {Derived} */ (active_reaction)
48+
: null;
49+
4550
/** @type {Derived<V>} */
4651
const signal = {
4752
children: null,
@@ -53,12 +58,11 @@ export function derived(fn) {
5358
reactions: null,
5459
v: /** @type {V} */ (null),
5560
version: 0,
56-
parent: active_effect
61+
parent: parent_derived ?? active_effect
5762
};
5863

59-
if (active_reaction !== null && (active_reaction.f & DERIVED) !== 0) {
60-
var derived = /** @type {Derived} */ (active_reaction);
61-
(derived.children ??= []).push(signal);
64+
if (parent_derived !== null) {
65+
(parent_derived.children ??= []).push(signal);
6266
}
6367

6468
return signal;
@@ -104,6 +108,21 @@ function destroy_derived_children(derived) {
104108
*/
105109
let stack = [];
106110

111+
/**
112+
* @param {Derived} derived
113+
* @returns {Effect | null}
114+
*/
115+
function get_derived_parent_effect(derived) {
116+
var parent = derived.parent;
117+
while (parent !== null) {
118+
if ((parent.f & DERIVED) === 0) {
119+
return /** @type {Effect} */ (parent);
120+
}
121+
parent = parent.parent;
122+
}
123+
return null;
124+
}
125+
107126
/**
108127
* @template T
109128
* @param {Derived} derived
@@ -113,7 +132,7 @@ export function execute_derived(derived) {
113132
var value;
114133
var prev_active_effect = active_effect;
115134

116-
set_active_effect(derived.parent);
135+
set_active_effect(get_derived_parent_effect(derived));
117136

118137
if (DEV) {
119138
let prev_inspect_effects = inspect_effects;
@@ -162,14 +181,13 @@ export function update_derived(derived) {
162181
}
163182

164183
/**
165-
* @param {Derived} signal
184+
* @param {Derived} derived
166185
* @returns {void}
167186
*/
168-
export function destroy_derived(signal) {
169-
destroy_derived_children(signal);
170-
remove_reactions(signal, 0);
171-
set_signal_status(signal, DESTROYED);
187+
export function destroy_derived(derived) {
188+
destroy_derived_children(derived);
189+
remove_reactions(derived, 0);
190+
set_signal_status(derived, DESTROYED);
172191

173-
// TODO we need to ensure we remove the derived from any parent derives
174-
signal.v = signal.children = signal.deps = signal.ctx = signal.reactions = null;
192+
derived.v = derived.children = derived.deps = derived.ctx = derived.reactions = null;
175193
}

packages/svelte/src/internal/client/reactivity/types.d.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,15 @@ export interface Reaction extends Signal {
2323
fn: null | Function;
2424
/** Signals that this signal reads from */
2525
deps: null | Value[];
26-
parent: Effect | null;
2726
}
2827

2928
export interface Derived<V = unknown> extends Value<V>, Reaction {
3029
/** The derived function */
3130
fn: () => V;
3231
/** Reactions created inside this signal */
3332
children: null | Reaction[];
33+
/** Parent effect or derived */
34+
parent: Effect | Derived | null;
3435
}
3536

3637
export interface Effect extends Reaction {
@@ -58,6 +59,8 @@ export interface Effect extends Reaction {
5859
first: null | Effect;
5960
/** Last child effect created inside this signal */
6061
last: null | Effect;
62+
/** Parent effect */
63+
parent: Effect | null;
6164
/** Dev only */
6265
component_function?: any;
6366
}

packages/svelte/src/internal/client/runtime.js

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,9 +767,24 @@ export function get(signal) {
767767
} else if (is_derived && /** @type {Derived} */ (signal).deps === null) {
768768
var derived = /** @type {Derived} */ (signal);
769769
var parent = derived.parent;
770+
var target = derived;
770771

771-
if (parent !== null && !parent.deriveds?.includes(derived)) {
772-
(parent.deriveds ??= []).push(derived);
772+
while (parent !== null) {
773+
// Attach the derived to the nearest parent effect, if there are deriveds
774+
// in between then we also need to attach them too
775+
if ((parent.f & DERIVED) !== 0) {
776+
var parent_derived = /** @type {Derived} */ (parent);
777+
778+
target = parent_derived;
779+
parent = parent_derived.parent;
780+
} else {
781+
var parent_effect = /** @type {Effect} */ (parent);
782+
783+
if (!parent_effect.deriveds?.includes(target)) {
784+
(parent_effect.deriveds ??= []).push(target);
785+
}
786+
break;
787+
}
773788
}
774789
}
775790

packages/svelte/tests/signals/test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,4 +739,30 @@ describe('signals', () => {
739739
assert.deepEqual(a.reactions, null);
740740
};
741741
});
742+
743+
test('nested deriveds clean up the relationships when used with untrack', () => {
744+
return () => {
745+
let a = render_effect(() => {});
746+
747+
const destroy = effect_root(() => {
748+
a = render_effect(() => {
749+
$.untrack(() => {
750+
const b = derived(() => {
751+
const c = derived(() => {});
752+
$.untrack(() => {
753+
$.get(c);
754+
});
755+
});
756+
$.get(b);
757+
});
758+
});
759+
});
760+
761+
assert.deepEqual(a.deriveds?.length, 1);
762+
763+
destroy();
764+
765+
assert.deepEqual(a.deriveds, null);
766+
};
767+
});
742768
});

0 commit comments

Comments
 (0)