fix: address derived memory leak on disconnection from reactive graph

fix-memory-leak-
Dominic Gannaway 1 year ago
parent 68263c8615
commit 1c6199fe48

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: address derived memory leak on disconnection from reactive graph

@ -323,6 +323,7 @@ export function prop(props, key, flags, fallback) {
from_child = true;
set(inner_current_value, value);
get(current_value); // force a synchronisation immediately
current_value.version++;
}
return value;

@ -204,6 +204,7 @@ export function check_dirtiness(reaction) {
if (dependencies !== null) {
var length = dependencies.length;
var is_equal;
var reactions;
for (var i = 0; i < length; i++) {
var dependency = dependencies[i];
@ -211,13 +212,13 @@ export function check_dirtiness(reaction) {
if (!is_dirty && check_dirtiness(/** @type {import('#client').Derived} */ (dependency))) {
is_equal = update_derived(/** @type {import('#client').Derived} **/ (dependency), true);
}
var version = dependency.version;
if (is_unowned) {
// If we're working with an unowned derived signal, then we need to check
// if our dependency write version is higher. If it is then we can assume
// that state has changed to a newer version and thus this unowned signal
// is also dirty.
var version = dependency.version;
if (version > /** @type {import('#client').Derived} */ (reaction).version) {
/** @type {import('#client').Derived} */ (reaction).version = version;
@ -228,7 +229,7 @@ export function check_dirtiness(reaction) {
// If we are working with an unowned signal as part of an effect (due to !current_skip_reaction)
// and the version hasn't changed, we still need to check that this reaction
// if linked to the dependency source otherwise future updates will not be caught.
var reactions = dependency.reactions;
reactions = dependency.reactions;
if (reactions === null) {
dependency.reactions = [reaction];
} else {
@ -238,6 +239,22 @@ export function check_dirtiness(reaction) {
} else if ((reaction.f & DIRTY) !== 0) {
// `signal` might now be dirty, as a result of calling `check_dirtiness` and/or `update_derived`
return true;
} else {
// It might be that the derived was was dereferenced from its dependencies but has now come alive again.
// In thise case, we need to re-attach it to the graph and mark it dirty if any of its dependencies have
// changed since.
if (version > /** @type {import('#client').Derived} */ (reaction).version) {
/** @type {import('#client').Derived} */ (reaction).version = version;
is_dirty = true;
}
reactions = dependency.reactions;
if (reactions === null || !reactions.includes(reaction)) {
if (reactions === null) {
dependency.reactions = [reaction];
} else {
reactions.push(reaction);
}
}
}
}
}
@ -422,7 +439,8 @@ function remove_reaction(signal, dependency) {
}
}
}
if (reactions_length === 0 && (dependency.f & UNOWNED) !== 0) {
debugger;
if (reactions_length === 0 && (dependency.f & DERIVED) !== 0) {
// If the signal is unowned then we need to make sure to change it to maybe dirty.
set_signal_status(dependency, MAYBE_DIRTY);
remove_reactions(/** @type {import('#client').Derived} **/ (dependency), 0);
@ -815,7 +833,7 @@ export function get(signal) {
) {
if (current_dependencies === null) {
current_dependencies = [signal];
} else {
} else if (current_dependencies[current_dependencies.length - 1] !== signal) {
current_dependencies.push(signal);
}
}

@ -2,13 +2,14 @@ import { describe, assert, it } from 'vitest';
import { flushSync } from '../../src/index-client';
import * as $ from '../../src/internal/client/runtime';
import {
destroy_effect,
effect,
effect_root,
render_effect,
user_effect
} from '../../src/internal/client/reactivity/effects';
import { source, set } from '../../src/internal/client/reactivity/sources';
import type { Derived, Value } from '../../src/internal/client/types';
import type { Derived, Effect, Value } from '../../src/internal/client/types';
import { proxy } from '../../src/internal/client/proxy';
import { derived } from '../../src/internal/client/reactivity/deriveds';
@ -423,4 +424,27 @@ describe('signals', () => {
destroy();
};
});
test('owned deriveds correctly cleanup when no longer connected to graph', () => {
let a: Derived<unknown>;
let state = source(0);
const destroy = effect_root(() => {
render_effect(() => {
a = derived(() => {
$.get(state);
});
$.get(a);
});
});
return () => {
flushSync();
assert.equal(a?.deps?.length, 1);
assert.equal(state?.reactions?.length, 1);
destroy();
assert.equal(a?.deps?.length, 1);
assert.equal(state?.reactions, null);
};
});
});

Loading…
Cancel
Save