Skip to content

Fix stale children committing when refresh() supersedes an async generator yield - #379

Open
brainkim wants to merge 1 commit into
mainfrom
fix/async-refresh-race
Open

Fix stale children committing when refresh() supersedes an async generator yield#379
brainkim wants to merge 1 commit into
mainfrom
fix/async-refresh-race

Conversation

@brainkim

Copy link
Copy Markdown
Member

Fixes the webkit failure discovered on #375.

The bug

When an async generator component calls refresh() before its next yield has committed, the yielded children are stale: the refresh immediately resumes the generator, and only the fresh children should commit.

async function* Component(this: Context) {
  await Promise.resolve();
  this.refresh();
  yield <span>Hello</span>;    // stale before it commits
  yield <span>Goodbye</span>;  // what should commit
}

Whether that held depended on a race between two unsynchronized microtask chains: the parent's commit walk vs. the enqueued run's diff. On V8 and older JavaScriptCore the enqueued diff won (Hello never appears — the behavior the cascades test asserts). On JSC as of Safari 26 the commit walk wins: Hello flashes into the DOM and render() resolves with it.

Root cause, measured

Pure-JS probe, no crank involved:

shape V8 old JSC (webkit-2191) new JSC (webkit-2336)
Promise.resolve().finally().then() tick 4 tick 4 tick 2
catch().finally().then() tick 5 tick 5 tick 3

JSC optimized .finally() by two microtask ticks. Crank's enqueue machinery (block.finally(advance), inflight[0].finally(runComponent)) sits exactly on those ticks, so the race flipped with no crank code changing. Generator resumption and await hops are identical across all three engines — it is .finally alone.

Main's CI cannot see this: playwright-test pins playwright-core 1.54 → webkit-2191. #375's CI installs current webkit → 2336, which is how it surfaced. It reproduces deterministically (3/3 locally), file run alone.

The fix

Make the supersede explicit instead of racing for it:

  • refresh() sets a new IsSupersededByRefresh flag when a run is already in flight — self-refresh origin only.
  • When the pending iteration arrives with the flag set (and not done), the stale children are not diffed at all, and the parent's diff resolves with the enqueued run's diff.
  • The block chain derives from the iteration via a closure rather than from the parent diff — the enqueued run only starts once the block settles, so deriving it from the now-chasing diff would deadlock.

Two constraints shaped this, both enforced by existing tests:

  • External updates must not chase. A re-render requested while a run is in flight coalesces into the enqueued run, but the in-flight run commits its own children ("for...of enqueues": p1 resolves showing "Hello 1" while p2–p5 coalesce). The flag keys on refresh origin for exactly this reason.
  • The promise graph must be unchanged when no supersede occurs. Earlier drafts added one microtask hop to every async-gen diff and broke tests whose assertions sample the DOM mid-flight. The final shape adds zero reactions on the clean path.

Verification

  • chromium (playwright-test, main's runner): 577 passed
  • webkit-2336, webkit-2191, chromium (libuild runner on the Replace playwright-test/uvu with the libuild test runner #375 branch with this crank.ts): 602 passed, 0 failed each
  • Instrumented MutationObserver trace: identical on V8 and new JSC — a single commit of the fresh children; the stale children never appear
  • tsc --noEmit clean, eslint clean

The 22 other webkit failures on #375 were fallout from this one (libuild's browser runner runs afterEach inside its try, so the failing test leaked a Sinon console.error stub into every later test that stubs — being reported to libuild separately).

Intended for a 0.7.11 patch release via release/0.7 as well; this is the main-line landing.

🤖 Generated with Claude Code

https://claude.ai/code/session_019pggktip8wsuxzy2VCY9p7

…yield

When an async generator component calls refresh() while its next iteration
is still pending, the yielded children are stale before they commit: the
refresh enqueues a run which immediately resumes the generator. Whether
the stale children ever reached the DOM depended on a race between two
microtask chains -- the parent's commit walk and the enqueued run's diff --
whose relative order is not specified. V8 and older JavaScriptCore
resolved the race one way (stale children never appear); JavaScriptCore
as of Safari 26 resolves .finally() two ticks faster than V8 (measured:
Promise.resolve().finally().then() lands on tick 2 vs tick 4), which
flips it: the stale children flash into the DOM and the render promise
resolves with them. Caught by "async generator refresh during await with
direct yield" in test/cascades.tsx on webkit 26.5, which playwright-test
pins too old to see.

The fix makes the supersede explicit instead of racing for it. refresh()
sets an IsSupersededByRefresh flag when a run is already in flight; when
the pending iteration then arrives, its children are not diffed at all
and the parent's diff resolves with the enqueued run's diff instead. The
block chain is derived from the iteration via a closure rather than from
the parent diff, because the enqueued run only starts once the block
settles -- deriving it from the now-chasing parent diff would deadlock.

The promise graph is unchanged whenever no supersede occurs. This
matters: external re-renders must coalesce without chasing (the
"for...of enqueues" test: the in-flight run commits its own children),
and several tests sample the DOM mid-flight and encode the exact
microtask hop counts of the current implementation.

Verified: 577 passed (chromium, playwright-test), 602 passed 0 failed on
webkit 2336 and webkit 2191 and chromium (libuild runner on the #375
branch with this crank.ts), and an instrumented trace shows identical
mutation order on V8 and new JSC: a single commit of the fresh children,
with the stale children never appearing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
brainkim added a commit that referenced this pull request Aug 12, 2026
Three backported fixes: the async generator self-refresh supersede race
(engine-dependent on Safari 26's faster .finally(); main PR #379), the
createElement props mutation fix (#356), and the jsx template adjacent
text line whitespace fix (#359).

Verified on this base: 573 passed, 3 skipped; test/jsx-tag.ts 29/29 run
explicitly. The built UMD bundle traces identically on chromium and
webkit 26.5: the superseded children never commit and render resolves
with the fresh ones.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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