From 638ab370a561f97552229589f2f334655426fe5c Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 27 May 2026 22:52:40 +0200 Subject: [PATCH] fix: prevent batch unlinking twice (#18303) Merged #18298 a bit too soon - there's still a situation which can corrput the linked list: If we merge one batch into another, it will unlink the batch immediately. But `discard` also calls it, running the unlink logic again, which can corrupt the linked list. Sketch: 1. Batch A is pending. 2. Batch B starts later, intersects A, resolves first, and merges into A. 3. B is unlinked immediately, but `A.oncommit(() => B.discard())` remains. 4. Independent batch C is created while A is still pending and is linked after A. 5. A commits, calls `B.discard()`, and B's stale `#prev`/`#next` can set `A.#next = null` / `last_batch = A`, disconnecting C. Not able to produce a failing test from it but it's definitely a fix we need to make. Also moved `#link` into a constructor, because it is (and should be) used only once. Also made the action after an error `discard` instead of just `#unlink` because this batch is done for and e.g. pending `settled` should resolve, too. --- .../src/internal/client/reactivity/batch.js | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 21e2cc6922..74448da03a 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -214,6 +214,18 @@ export class Batch { #decrement_queued = false; + constructor() { + // link batch + if (last_batch === null) { + first_batch = last_batch = this; + } else { + last_batch.#next = this; + this.#prev = last_batch; + } + + last_batch = this; + } + #is_deferred() { if (this.is_fork) return true; @@ -327,11 +339,11 @@ export class Batch { } catch (e) { reset_all(root); // If there's no async work left, this branch is now dead and needs - // to be unlinked to not become a zombie that is never cleaned up. + // to be discarded to not become a zombie that is never cleaned up. // See https://github.com/sveltejs/svelte/issues/18221#issuecomment-4497918414 // for a (non-minimal) reproduction that demonstrates a case where this is necessary // to not get follow-up false-positives via "batch has scheduled roots" invariant errors. - if (!this.#is_deferred()) this.#unlink(); + if (!this.#is_deferred()) this.discard(); throw e; } } @@ -852,7 +864,6 @@ export class Batch { static ensure() { if (current_batch === null) { const batch = (current_batch = new Batch()); - batch.#link(); if (!is_processing && !is_flushing_sync) { queue_micro_task(() => { @@ -972,18 +983,11 @@ export class Batch { this.#roots.push(e); } - #link() { - if (last_batch === null) { - first_batch = last_batch = this; - } else { - last_batch.#next = this; - this.#prev = last_batch; - } - - last_batch = this; - } - #unlink() { + // #merge calls #unlink, discard later on does it again - prevent + // running it multiple times to not corrupt the linked list + if (!this.linked) return; + var prev = this.#prev; var next = this.#next;