Skip to content

fix(signals): preserve sibling queues during traversal - #2947

Merged
ryansolid merged 2 commits into
solidjs:nextfrom
brenelz:fix/scheduler-child-queue-removal
Jul 28, 2026
Merged

fix(signals): preserve sibling queues during traversal#2947
ryansolid merged 2 commits into
solidjs:nextfrom
brenelz:fix/scheduler-child-queue-removal

Conversation

@brenelz

@brenelz brenelz commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

The bug

Queue.run() traverses child queues with a monotonically increasing array index. A boundary effect can dispose its owner while its queue is running, which removes that queue from _children. The next sibling then shifts into the current index and is skipped, leaving its already-queued effects unexecuted with no later flush scheduled.

The fix

Advance the traversal index only while the child that just ran still occupies that slot. If it removed itself or an earlier sibling, process the child that shifted into the current index before advancing.

A regression test creates two loading-boundary queues, disposes the first from its effect, and verifies that the second effect still runs in the same flush.

Tests

  • vitest run: 78 files, 1,147 tests passed
  • tsc -p tsconfig.build.json --noEmit
  • Prettier check for the changed source and test
  • Changeset status validates an @solidjs/signals patch release

@changeset-bot

changeset-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 09d2540

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@solidjs/signals Patch
test-integration Patch
solid-js Patch
babel-preset-solid Patch
@solidjs/web Patch
@solidjs/html Patch
@solidjs/h Patch
@solidjs/universal Patch
@solidjs/element Patch
hackernews-spa-example Patch
hackernews-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed-hq

codspeed-hq Bot commented Jul 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 117 skipped benchmarks1


Comparing brenelz:fix/scheduler-child-queue-removal (09d2540) with next (63cd066)

Open in CodSpeed

Footnotes

  1. 117 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@ryansolid
ryansolid merged commit 21ff279 into solidjs:next Jul 28, 2026
4 of 5 checks passed
ryansolid added a commit that referenced this pull request Jul 28, 2026
…osal

#2947 recovered the cursor when a child removed itself or one earlier
sibling, but a single dispose can remove several sibling queues at once
(a root owning more than one boundary). The list then shifts by more
than a slot and the cursor lands past an unprocessed later sibling,
whose effects never run.

Stamp each child with the current pass before running it, so the
traversal is idempotent and a shift can be recovered by rescanning from
the front. Children appended mid-pass keep running in that pass, as
before.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid

Copy link
Copy Markdown
Member

Merged — thanks, the diagnosis was right and the regression test is a good one.

One thing worth flagging for the record: the fix as written recovers the cursor for one removal, but a single dispose() can remove several sibling queues at once — a root owning more than one boundary removes them all from the same parent's _children. The list then shifts by more than a slot and the cursor lands past an unprocessed later sibling, which is the same symptom the PR fixes:

[Q_a1, Q_a2, Q_b]   // Q_a1 and Q_a2 belong to root A, Q_b to root B
                    // an effect in Q_a2 disposes root A -> [Q_b]
                    // cursor was at 1; _children[1] is now undefined, so it
                    // doesn't advance -> length is 1 -> Q_b never runs

I've landed a follow-up on next (1017393) that generalises it: each child is stamped with the current pass before it runs, so the traversal is idempotent and a shift can be recovered by rescanning from the front — every child still runs exactly once, and children appended mid-pass still run in that pass. Your test plus a three-queue variant both cover it now.

Costs ~30 B brotli in the +createStore size scenario, which is a fair trade for closing the case properly.

ryansolid added a commit that referenced this pull request Jul 28, 2026
The scenario measured 12.59 KB and had already drifted past the 12.5 KB
cap before this batch of fixes. The overage is accumulated correctness
work on paths createStore always retains — reconcile's container-kind
guard (#2946), the projection derive-swap path (#2941), and the queue
traversal pass stamp (#2947) — not a shakeable feature. Restores the
~2% headroom the sibling scenarios carry.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants