Skip to content

fix(signals): a direct-commit reader is neither entered nor born held — until() after a staged frame (#3482) - #3490

Closed
ryansolid wants to merge 1 commit into
nextfrom
fix/until-born-held-3482
Closed

ryansolid wants to merge 1 commit into
nextfrom
fix/until-born-held-3482

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Closes #3482.

Bug

yield until(predicate) inside an action() never resolved when the predicate read the optimistic store a live stream feeds and the confirming frame landed before until() was called — the natural ordering for a server that broadcasts to subscribers before it answers the mutation. With a timeout, TimeoutError; without one the action hung forever. This is the single-primitive shape the until() docstring recommends.

Regression from d80cd1f6 (#3451, A29 creation-time born-held). 2.0.0-rc.8 is unaffected; next fails through c54e4c0ca.

Cause

A reader created mainline under a hold is born held (enterStagedRead sets stagedEntry; recompute stamps it, skips its synchronous first run and adds it to the transaction's _gatedSubs to replay at the commit). For a promise-delivery effect the commit is the action's settle — which its own promise is holding open. recompute's DIRECT_COMMIT arm exists for exactly this case ("deadlocks the hold (until)") but is gated on bornHeld === null, so the born-held path pre-empted it.

Fix

enterStagedRead's mainline arm exempts CONFIG_DIRECT_COMMIT readers — resolve(), until(), awaitable refresh()'s waiter — beside the existing verdict-pull exemption. They read staged truth by contract (the tunnel that keeps a hold deadlock-free) and apply on their own microtask, not the transaction's stashed queues, so they neither enter the transaction nor are born held. stagedEntry never gets set for them; every downstream bornHeld === null holds and they take the existing direct-commit path. The predicate sees the held frame, the action settles, the commit reveals the frame with the overlay revert.

-    if (GlobalQueue._verdictPull) return;
+    if (GlobalQueue._verdictPull || ctx._config & CONFIG_DIRECT_COMMIT) return;

Tests

tests/until-held-frame-before-call.test.ts — the issue's reproduction (frame lands while the action awaits its answer; until() reached after) and the control ordering (until() subscribed first, then the frame). On next the first fails with TimeoutError and the control passes; with the fix both pass.

signals 2,735 (+2) / 2 expected fails unchanged; solid 620; web 825 / 986 / 185 against the rebuilt dist. until, until-entanglement, born-held, resolve, refresh-await, tree-shake floor and rules index green.

Size

~+20 B minified on a core-retained read path; read +20..+40 B brotli across the scenarios (−30 on observe — layout). Two app caps (hydrating (no stores), CSR with …) ratcheted 0.05 KB with notes; the in-package tree-shake floor did not move.

Reproduction, bisect and analysis by @brenelz.

…#3482)

`yield until(predicate)` inside an action never resolved when the
predicate read the optimistic store a live stream feeds AND the confirming
frame had landed before until() was called — the natural ordering for a
server that broadcasts to subscribers before it answers the mutation. With
a timeout, TimeoutError; without one the action hung forever.

Regression from d80cd1f (#3451): a reader created mainline under a hold
is born held — its synchronous first run is skipped and it is added to the
transaction's _gatedSubs to replay at the commit. For a promise-delivery
effect the commit IS the action's settle, which its own promise is holding
open. recompute's DIRECT_COMMIT arm exists for exactly this case but is
gated on `bornHeld === null`, so the born-held path pre-empted it.

enterStagedRead's mainline arm now exempts CONFIG_DIRECT_COMMIT readers
(resolve(), until(), awaitable refresh()'s waiter) beside the verdict-pull
exemption: they read staged truth by contract — the tunnel that keeps a
hold deadlock-free — and apply on their own microtask, so they neither
enter the transaction nor are born held. stagedEntry never gets set for
them; every downstream `bornHeld === null` holds and they take the existing
direct-commit path. The predicate sees the held frame, the action settles,
and the commit reveals the frame with the overlay revert.

Test: tests/until-held-frame-before-call.test.ts — the issue's reproduction
(frame lands while the action awaits its answer; until() reached after) and
the control ordering (until() subscribed first). The first fails on next
with TimeoutError, the control passes on both.

Size: ~+20 B minified on a core-retained path; +20..+40 B brotli across the
scenarios (−30 on observe — layout). Two app caps ratcheted 0.05 KB with
notes. Tree-shake floor unchanged.

Reproduction, bisect and analysis by @brenelz (#3482).

Co-authored-by: Brenley Dueck <brenelz@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a2297c7

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
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
solid-js Patch
@solidjs/universal 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

codspeed Bot commented Sep 16, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 175 untouched benchmarks


Comparing fix/until-born-held-3482 (a2297c7) with next (c54e4c0)

Open in CodSpeed

@ryansolid

Copy link
Copy Markdown
Member Author

Closing unmerged — superseded by #3491. Not a bug: the until(...) in yield until(...) is evaluated in the await continuation, before the yield re-enters the transaction, so the effect is created mainline and born held as A29 specifies. A bare yield; after the await puts the call inside the transaction, where recompute's existing DIRECT_COMMIT arm — scoped to the effect's own held transition — already handles it; verified on next in #3491.

The exemption here was also too broad: CONFIG_DIRECT_COMMIT covers resolve() and awaitable refresh()'s waiter, which are routinely called outside actions. From mainline against a foreign hold they would have served that action's uncommitted frame instead of the committed view — a leak, not a fix. The gap it exposes (nothing pins that posture for the direct-commit readers) gets its own tests-only PR.

Claude via Cursor

@ryansolid ryansolid closed this Sep 16, 2026
github-actions Bot pushed a commit to yumemi-thomas/solid that referenced this pull request Sep 16, 2026
…rame (solidjs#3482)

until() and resolve() (CONFIG_DIRECT_COMMIT) created after a confirming
frame was staged under an action's optimism:

- inside the action after a bare `yield` — the action's own reader; the
  DIRECT_COMMIT arm delivers under its own hold and the action settles
  (the documented form, solidjs#3491);
- inside the action from an `await` continuation with no bare `yield` —
  a mainline reader over its own hold: born held, replays at the commit
  its promise holds open, times out (solidjs#3482 as filed);
- outside the action — a mainline reader over a FOREIGN hold: pending
  through the hold, served the committed view after the commit, never
  the unrevealed frame.

The third is the pin the suite lacked: the exemption proposed in solidjs#3482
and declined in solidjs#3490 passed 2,733 tests while making a mainline
resolve() serve another action's speculative frame. Verified: with that
one-liner applied the second and third fail; on next all three pass.

Rules index regenerated (the file cites A29).

Co-authored-by: Claude <noreply@anthropic.com>
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.

1 participant