Skip to content

Fix: SendCommand raced Tick against the command, corrupting state on WebPlayer#1837

Merged
alexwarren merged 2 commits into
mainfrom
fix/sendcommand-tick-race
Jul 9, 2026
Merged

Fix: SendCommand raced Tick against the command, corrupting state on WebPlayer#1837
alexwarren merged 2 commits into
mainfrom
fix/sendcommand-tick-race

Conversation

@alexwarren

Copy link
Copy Markdown
Contributor

Summary

  • SendCommand launched Tick(0) and HandleCommandAsyncInternal concurrently (both fire-and-forget) whenever elapsedTime > 0. Both read/mutate the same Elements/Fields collections, so on WebPlayer's real thread pool this could corrupt a Dictionary mid-turn ("Operations that change non-concurrent collections must have exclusive access"), which then throws on every subsequent access until the process restarts. WasmPlayer's single-threaded runtime can't manifest true corruption, so it looked fine there.
  • This race predates Fix: timer created inside wait/pause/ask/menu callback never fires #1835 but was rarely reachable — Fix: timer created inside wait/pause/ask/menu callback never fires #1835 restored elapsed-time tracking after wait/pause/ask/menu resolve, so elapsedTime > 0 became the common case for any wait-heavy game, turning a latent race into a reliably reproducible one (reported against the game "Oracle Feels Confused", which leans heavily on wait { }).
  • Fix: Tick(0) now runs to completion before the command starts, instead of racing it.

Test plan

  • New regression test SendCommand_ElapsedTimeTriggersTimer_TimerRunsBeforeCommand (tests/EngineTests/CallbackTests.cs), backed by a new timer/command pair in callbacktest.aslx
  • Confirmed the test fails on the pre-fix code (command output landed before the timer's) and passes with the fix
  • Full solution test suite passes (dotnet test --configuration Release, 319/319)

🤖 Generated with Claude Code

alexwarren and others added 2 commits July 9, 2026 17:13
…WebPlayer

When elapsedTime > 0, SendCommand launched Tick(0) and HandleCommandAsyncInternal
concurrently (both fire-and-forget). Both read/mutate the same Elements/Fields
collections, so on WebPlayer's real thread pool this could corrupt a Dictionary
mid-turn ("Operations that change non-concurrent collections must have exclusive
access"), which then throws on every subsequent access until the process restarts.
WasmPlayer's single-threaded runtime can't manifest true corruption, so it looked
fine there.

This was reachable but rare until #1835 restored elapsed-time tracking after
wait/pause/ask/menu resolve, making elapsedTime > 0 the common case for any
wait-heavy game and turning a latent race into a reliably reproducible one.

Now Tick(0) runs to completion before the command starts, instead of racing it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@alexwarren alexwarren enabled auto-merge (squash) July 9, 2026 17:02
@alexwarren alexwarren merged commit 69bb330 into main Jul 9, 2026
3 checks passed
@alexwarren alexwarren deleted the fix/sendcommand-tick-race branch July 9, 2026 17:03
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