Skip to content

fix(signals): second write while an async chain is in flight (#3373, #3374, #3375, #3376) - #3381

Merged
ryansolid merged 2 commits into
nextfrom
fix/async-chain-supersession
Sep 12, 2026
Merged

ryansolid merged 2 commits into
nextfrom
fix/async-chain-supersession

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Fixes #3373, #3374, #3375, #3376 — the four reports from @GabbeV that share one shape: a second write arrives while an async chain started by the first is still in flight, and the engine's hold accounting loses track of the flight.

Each was reproduced at the @solidjs/signals level with a manual clock (packages/signals/tests/async-chain-supersession.test.ts, seven pins) before fixing. All reproduce on next @ 344ed054.

Root causes and fixes

#3373 / #3376 — a stale first-hop landing commits the newer signal

a = memo(delay(count)), b = memo(delay(a)). count=1 starts a1; a1 lands and starts b1; count=2 arrives and starts a2 while b1 is still up. a2 only changes a's status (not yet its value), so b is marked pending on a by propagation with b1 still current — nothing supersedes it. When b1 lands, asyncWrite's clearStatus(b) wiped all of b's pending sources, including a. b's staged 1 woke the text effect, which saw nothing pending; the transaction's reporter for a found nothing blocking; it committed count=2, b=1 with a2 in the air → 2 / 1 (#3373), and isPending(b) read false for the gap (#3376).

Fix: the landing uses landStatus, which retires only the node's own entry. With another source still pending the node stays derivatively pending on it; the landed value is written and held (it is still the answer for the inputs it was asked with), and the input's settle releases it — or its value change re-asks the node. Pinned both ways: a2 landing changed (reveal 2 / 2 at b2) and unchanged (b1's held answer reveals at a2's landing, no extra flight).

Complement at registration: handleAsync now drops pending entries the node's inputs propagated earlier. The run that asked the flight read every input without throwing, so a still-pending input was masked for it (an active override, A17) and does not describe the new answer. Without this the A17 spec pin regressed (the override-derived flight was held by the authoritative refetch).

#3375Loading on={page} reveals content with details still pending

Two mechanisms:

  1. Boundary collection. The on reset clears _sources and rebuilds from later notifications. notifyStatus dedupes on the effect's _pendingSources, so a source the effect already carried across the reset (details, from the count write's flight) is never re-reported, and details' re-flight after pageData2 lands stays invisible. When pageData settled, _sources emptied and the boundary revealed with the effect still pending on details. Fix: a collecting boundary records every source the notifying effect is pending on (node._x._pendingSources), gated to the STATUS_PENDING type — an error boundary must not collect pending sources, since _checkSources would hold its fallback on them.

  2. Transaction completion. transitionComplete tested "the source's own flight is up" with _error.source === source. Propagation from an input that went pending later overwrites _error with the input, so details — still flying — read as settled and the count=1 hold committed at 5500, ahead of its answer. Fix: the test is the source's self entry in _pendingSources, which only notifyStatus's source path adds and the landing / supersede sweep retire.

Note for the reviewer: the pinned timeline records page=2 riding the count hold (Sum: 3 @8500). That is A15 applying — pageData, a reader of page, lives in the hold since its earlier landing joined it — where before it was leaking early through the _error.source glitch. If an on reset should release the page write instead, that is a spec ruling, not this bug.

#3374 — a same-value rewrite after a keyed remount publishes early

count=1 is held with details in flight. <Show keyed when={version()}> remounts the reader; the new reader is served the committed Details: 0 by the A15 reveal carve-out (correct — the frame still shows Count: 0) and recorded for replay. But the transaction's only reporter for details was the disposed reader. The same-value setCount(1) activates the transaction; transitionComplete finds no live reporter, drops the source and commits count=1 with details1 still up → Count: 1 beside Details: 0 for a second.

Fix: a carve-out reader joins the transaction's reporters for the node when it already has an entry (heldFromStale, one Map lookup). It observes the flight (A15: async work observed by a reader settles as one unit with the writes that asked it) and dies with disposal like any reporter (reporterBlocksSource sees the dep the read linked). INV-3 is preserved as "entries open only from queue notification" — a boundary-consumed flight has no entry and stays consumed; the internals doc is amended accordingly. A chain's intermediate memo never reaches the carve-out (the read re-pulls it and enters the transaction), and a node that is both an origin and pending on an upstream re-ask blocks through its own flight until it lands — both pinned.

Size

Core floor 22,648 → 22,780 (+132 B minified) after a golf pass (from +259 on the first cut). Brotli scenarios +32…+73 B; nine .size-limit.js caps bumped with notes; size-limit exits 0.

Verification

  • signals 1758/1758 (7 new pins), solid 595/595, web 734/734; tsc clean on src/ and the new test (remaining tsc errors are pre-existing test files on next).
  • Every spec-async-semantics pin holds, including A17 (the registration clear exists for it).
  • Changeset: .changeset/fix-async-chain-second-write.md (@solidjs/signals, patch).
  • Docs: INTERNALS-ASYNC-STATE.mdlandStatus vs clearStatus, transitionComplete's own-flight test, INV-3's join site, the carve-out paragraph's "does not entangle" qualified.

The one edit with blast radius beyond the four repros is the transitionComplete own-flight test; suites are clean, but it is the line to watch on CI/CodSpeed.

Claude via Cursor

…3374, #3375, #3376)

A flight's landing retires only the node's own pending entry (`landStatus`).
An input re-asked mid-flight (`a` restarted while `b`'s first flight was up)
marks `b` pending on `a` by propagation with `b`'s flight still current —
nothing superseded it — and the full `clearStatus` at landing wiped that
entry: the stale answer read as settled, the transaction's reporter for `a`
found nothing pending below it, and the newer signal committed beside the
older derived value (`2 / 1`, #3373); `isPending(b)` blipped false (#3376).
The landed value is still written and held; `a`'s settle releases it or its
value change re-asks `b`. Complement at registration: a fresh flight drops
entries its inputs propagated earlier — the run read them, so a pending input
was masked for it (an active override, A17) and does not describe the answer.

`transitionComplete` tests a source's own flight by its self entry in
`_pendingSources`, not `_error.source`, which a later-pending input overwrites
on propagation while the flight is still in the air — the held write no longer
commits ahead of its answer once the load is re-asked under an `on`-scoped
boundary (#3375).

A collecting `Loading` boundary records every source the notifying effect is
pending on, not only the one the notification carries. Status propagation
dedupes on the effect's `_pendingSources`, so a source carried across an `on`
reset was never re-reported and its re-flight stayed invisible — the boundary
revealed when its one collected source settled with the effect still pending
(#3375).

A stale reader served a pending node's committed value by the reveal
carve-out (`heldFromStale`) joins the transaction's reporters for that node
when it has an entry: it displays the pre-flight value, so a keyed remount
that disposes the reader that opened the entry no longer lets a same-value
rewrite commit the held write while the derivation is in flight (`Count: 1`
beside `Details: 0`, #3374). Entries still open only from queue notification
(INV-3); a boundary-consumed flight stays consumed.

Core floor 22,648 -> 22,780 (+132 B); size caps bumped with notes.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Sep 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a82d2e3

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

@coveralls

coveralls commented Sep 12, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 34688159512

Coverage remained the same at 71.842%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1007
Covered Lines: 772
Line Coverage: 76.66%
Relevant Branches: 790
Covered Branches: 519
Branch Coverage: 65.7%
Branches in Coverage %: Yes
Coverage Strength: 15.02 hits per line

💛 - Coveralls

@codspeed

codspeed Bot commented Sep 12, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by ×3.6

⚡ 1 improved benchmark
✅ 159 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
projection derive: write one NESTED field (reference) 809.1 µs 222.5 µs ×3.6

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing fix/async-chain-supersession (a82d2e3) with next (344ed05)

Open in CodSpeed

…ts readers observed (#3375)

Ruled 2026-09-12: fallback-caught async holds nothing, in both orders. A
reader registered as a reporter while its boundary showed content stops
blocking once the boundary's `on` changes and it flips to the fallback
(`reporterBlocksSource` walks the reporter's queue chain); if nothing outside
the boundary consumes the flight, the hold is over. A reader outside the
boundary still holds.

The flush judges only the active transaction, so the reset wakes parked
transactions (`wokenTransitions`, deduped) and the flush re-enters a woken
one from the finally of an idle pass (`!scheduled`), adopting no unrelated
ambient work. Pins: the #3375 frames now release count=1 at the reset and
page=2 commits plain; a companion pin keeps the hold with an outside reader.

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

Copy link
Copy Markdown
Member Author

Pushed a82d2e3 on top: the A15 question flagged in the description is now ruled and implemented.

Ruling (2026-09-12): a Loading boundary's on reset ends the hold on writes that only its readers observed. A reader registered as a reporter while the boundary showed content stops blocking once the boundary flips to its fallback; if nothing outside the boundary consumes the flight, the hold is over. A reader outside the boundary still holds it.

Mechanism:

  • reporterBlocksSource walks the reporter's _queue._parent chain; a reporter behind a collecting pending-type boundary (_collectionType & STATUS_PENDING && !_initialized) is not live. Exact, because a collecting boundary consumes new notifications — the only reporters under it are the stale forwarded ones.
  • The flush judges only the active transaction, so the reset itself wakes parked transactions (wokenTransitions, deduped). The flush re-enters a woken transaction from the finally of an idle pass (!scheduled), so it adopts no unrelated ambient work; a wake with other live reporters simply re-parks.

#3375 pin now: 0: Loading | Sum: 03000: content | Details: 04000: Loading... | Sum: 1 | Sum: 25500: Sum: 38500: content | Details: 3. The two Sum publishes at 4000 are one synchronous drain: the ambient page=1 commit, then the released count=1 in the idle pass after it — no tear (fallback is showing). A companion pin keeps the hold when an outside reader also observes details.

While writing that companion pin I found a pre-existing #3322 artifact, unchanged by this PR: an effect computed mainline in the pass a stamped recompute later pulls into the transaction publishes its stale slot at commit before the contested re-derive publishes the right value (Sum: 1 then Sum: 2 in the same frame). The note in recompute says the stale value is never published; here it is. Tracked separately; the pin asserts around it.

Floor: +135 B on top (22,780 → 22,915); size caps bumped with notes. Existing tests untouched; signals 1759, solid 595, web 734.

Claude via Cursor

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