Fix coop multi-stage transition: host crash/hang on next stage (TFTD colony) - #184
Merged
Merged
Conversation
…y crash) A 2-stage co-op mission (TFTD alien colony assault, cargo/cruise ship, artifact site, the T'leth finale) crashed or hung the host when the last alien of a stage was killed. BattlescapeState::finishBattle's coop-host next-stage branch rebuilt the next stage (bgen.nextStage()) and popped the finished BattlescapeState, but never entered a new one: SavedBattleGame::_battleState was left pointing at the freed state. Because SavedBattleGame::getBattleGame() delegates through _battleState, the coop pump (connectionTCP::updateCoopTask -> BattlescapeGame::handleStateCoop) dereferenced the dangling BattlescapeGame every frame -> use-after-free crash (0xC0000005; byte-exact symbolicated against the shipped nightly PDB). On runs where the freed memory survived a frame the host instead hung forever on the "please wait" dialog while only the client (which loads the shipped blob) advanced to the next stage. Fix: after shipping the rebuilt stage to the client (setupCoop -> sendMissionFile, unchanged), the host now enters its OWN rebuilt battle by wiring a fresh BattlescapeState (setBattleState + NextTurnState), mirroring BriefingState::btnOkClick. The in-battle co-op turn handshake then syncs both machines into the next stage together. Belt-and-suspenders: ~BattlescapeState clears _save->_battleState when it still names the state being freed, so the coop pump fails safe on every teardown path. Repro/regression: tools/coop_test/test_coop_nextstage_crash.py resumes the player's scrubbed mid-battle save, kills the last stage-1 alien and asserts both machines enter STR_ALIEN_COLONY_P2. RED on the unfixed exe (host crash), GREEN with the fix (host coopTurn=2, client coopTurn=3). Skips cleanly where TFTD data is not staged (e.g. CI). The debrief-sync suite is unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
NonPolynomialTim
enabled auto-merge (squash)
September 4, 2026 04:25
NonPolynomialTim
added a commit
that referenced
this pull request
Sep 8, 2026
…-proof the gate
Two follow-ups from validating the previous commit against CI.
1. chkBattleStage is now stamped ONLY for multi-stage missions.
A single-stage battle - every UFO/terror/base mission, and every parallel-turn
fixture - now puts no extra field on the wire at all, so its next_turn is
byte-identical to before the guard existed. The guard only ever mattered for a
stage transition.
The predicate is "is this ANY stage of a multi-stage chain": the deployment has a
nextStage, OR some deployment's nextStage names it. Both halves are needed, and
the second is the non-obvious one. The LAST stage of a chain carries no nextStage
of its own (STR_ALIEN_COLONY_P2, STR_TLETH_P3, STR_MARS_THE_FINAL_ASSAULT) - and
the host is exactly the machine sitting on that last stage while the peer is still
loading the previous one. A plain "has a nextStage" test would leave the HOST
unstamped precisely when the peer needs the marker, reinstating the false alarm.
attachBattleChecksum and verifyBattleChecksum have one caller each (NextTurnState
and the next_turn handler), so this runs once per turn boundary and the deployment
sweep needs no cache.
Regression unchanged and still green: test_coop_nextstage_census races the host's
stage-2 turn screen against the client's load and still logs the skip
("the peer is on stage 'STR_ALIEN_COLONY_P2', this machine is on
'STR_ALIEN_BASE_ASSAULT'") with zero desync bundles on either machine.
2. test_parallel_heavy_death_repro no longer gates the suite.
It is a REPRO TOOL, not a guard - its own docstring says so - and its exit codes
are the reverse of what a gate assumes:
exit 0 = REPRO FIRED (the heavy-alien-death desync reproduced)
exit 3 = NO REPRO (every alien side stayed in census - i.e. clean)
exit 2 = harness/setup error
The runner scores exit 0 as PASS, so gating on it counts the bug FIRING as success
and a clean run as failure. Every green run of it on main (30 Aug run 33331963166,
4 Sep run 33857882577, and #184's over-budget entry, which the runner only reports
when rc==0) was green because the drift fired - exactly the signal a green pipeline
hides. It has been this way since it landed in #166 on 2026-08-30; the exit codes
have never been modified.
It still reproduces intermittently because the #166 fixes sit behind a lever that
defaults off (g_wireOrderState) pending the battlescape rewrite, so this is the
known state and not a regression. Quarantined: it runs and prints its verdict but
does not gate. Its rc=0 deserves an alert of its own - it is currently the only
automated thing that notices the drift firing - but that is a separate change.
Also verified along the way: coop-tests and build-linux are BOTH red on unmodified
main (control dispatch of ci-validate on f8b0834), so the gate was not failing
because of this branch.
NonPolynomialTim
added a commit
that referenced
this pull request
Sep 8, 2026
… it false-alarming (#188) Follow-up to #184. Both machines reached stage 2 of a co-op multi-stage mission, but the host could not see any aliens there while the client could - "the client was seeing aliens where I saw nothing". 1. The host never finished starting the stage. BattlescapeGenerator::nextStage() ends in resetTurnCounter(), parking the rebuilt save at turn 0 with _beforeGame = true. Vanilla clears that in SavedBattleGame::startFirstTurn(), reached via BriefingState::btnOkClick -> InventoryState -> OK; co-op shows neither screen on this path. So the host was left mid-initialisation: TileEngine::calculateLineVoxel excludes EVERY unit from line-of-sight while _beforeGame is up, so the host could never spot an alien; resetUnitTiles() never ran, so tiles were not matched to units and the player's own units kept nextStage()'s setVisible(false); and turn stayed 0, which silences the host's click_close/next_turn packets (both gated on turn >= 1). The client escaped all of it because it enters through SavedBattleGame::load(), whose tail is resetUnitTiles() + recalculateFOV(). A fresh mission start is unaffected for the same reason - there the host re-loads its own "battlehost" blob via CoopState::loadWorld, so it goes through the loader too. Fix: startFirstTurn() + recalculateFOV() after bgen.nextStage() and BEFORE setupCoop()/sendMissionFile(), so the blob shipped to the client is the state the host actually plays. before host beforeGame=true turn=0 spotted=[] discoveredFloor=242 client beforeGame=false turn=0 spotted=[1000052] discoveredFloor=988 after both beforeGame=false turn=1 spotted=[1000052] discoveredFloor=988 2. That exposed a latent false alarm in the drift tripwire. A transition hands the peer a whole new battle, and the host enters it while the peer is still downloading and loading the blob. A next_turn closed in that window compares the host's stage-2 terms against the peer's stage-1 state, so all three chkBattle* terms differ - they describe two different battles, not two disagreeing copies of one (playtest: peer itemId 351/turn 13/44 units vs host 463/turn 1/69 units). Both machines showed CO-OP DESYNC DETECTED at every transition. Previously unreachable because the host sat at turn 0 and never sent the packet. Fix: attachBattleChecksum stamps chkBattleStage and verifyBattleChecksum returns early when the peer's stage differs - the same "not comparable in this window" discipline as the neighbouring corpseReplayPendingAny()/rxPassDeferred() guards. Scoped to multi-stage missions only, so a single-stage battle's next_turn is byte-identical to before. The predicate is "any stage of a chain" (has a nextStage, or is some deployment's nextStage): the LAST stage has none of its own, and the host sits on exactly that stage while the peer loads the previous one, so "has a nextStage" alone would leave the host unstamped when it matters. PROTOCOL.md updated in the docs repo. 3. CI: build-linux could not install its dependencies. Debian 11 is end of life. bullseye-security stopped being published on 2026-08-31 and its Release file expired 2026-09-07 21:13 UTC, so apt-get update failed outright; before that it 404'd pool files its own index still advertised. The retry loop cannot help - neither is a mirror flake. Plain bullseye is frozen, carries no Valid-Until and is already on archive.debian.org, so point there and drop the dead security suite. Every package installed exists there; none ship inside the AppImage. Keeps the glibc 2.31 floor - debian:12 would fix apt too but drops Ubuntu 22.04 LTS, Mint 21 and RHEL/Rocky 9, which is a player support decision. Applied to BOTH ci-main.yml and ci-validate.yml (the PR gate). 4. test_parallel_heavy_death_repro no longer gates the suite. It is a repro tool, not a guard, and its exit codes are the reverse of what a gate assumes: exit 0 = the heavy-alien-death desync REPRODUCED, exit 3 = clean. The runner scores 0 as PASS, so every green run of it on main was green because the drift fired. It has been that way since it landed in #166. The #166 fixes sit behind a lever that defaults off (g_wireOrderState) pending the battlescape rewrite, so it still reproduces intermittently. Quarantined: runs and prints its verdict, does not gate. Tests: tools/coop_test/test_coop_nextstage_census.py asserts the stage-2 census, map fingerprint, fog and spotted-alien sets match on both machines, RACES the host's turn screen against the client's load and asserts zero desync bundles, and with NEXTSTAGE_FINISH=1 clears stage 2 and asserts an identical debriefing and return to the geoscape. Measured on the forced window - without the stage guard: 2 bundles; with it: 0. battle_state gains a beforeGame readout. Merged with build-linux temporarily removed from the required checks: its fix is in this PR but pull_request_target evaluates workflows from the base branch, so it could not go green until it was on main. Verified beforehand that coop-tests and build-linux were BOTH red on unmodified main (control dispatch on f8b0834), so the gate was not failing because of this branch. Required checks restored immediately after the merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Player report (TFTD 2.0.30 nightly, SEPARATE coop): in a 2-stage
STR_ALIEN_BASE_ASSAULT(alien colony), after clearing stage 1, killing the last alien crashes the host; if the client kills the last alien, the host hangs on "Please wait…". This affects every 2-stage TFTD mission (alien colony, cargo/cruise ship, artifact site, the T'leth finale).Root cause (byte-exact symbolicated vs the shipped nightly PDB)
BattlescapeState::finishBattle's coop-host next-stage branch rebuilt the next stage (bgen.nextStage()) and popped the finishedBattlescapeState, but never entered a new one.SavedBattleGame::_battleStatewas left pointing at the freed state;getBattleGame()delegates through it, so the every-frame coop pump (updateCoopTask → handleStateCoop) dereferenced the danglingBattlescapeGame→ use-after-free (0xC0000005). Heap-timing decides the symptom: sometimes it self-crashes (host-kill), sometimes the freed memory survives a frame and the host just hangs on the "please wait" dialog while only the client (which loads the shippedbattlehostblob) advances. The client side already worked — only the host was broken.Fix
After shipping the rebuilt stage to the client (
setupCoop → sendMissionFile, unchanged), the host now enters its own rebuilt battle —new BattlescapeState+spawnFromPrimedItems()+setBattleState()+NextTurnState, mirroringBriefingState::btnOkClick. The in-battle coop turn handshake then syncs both machines into the next stage together.Belt-and-suspenders:
~BattlescapeStateclears_save->_battleStatewhen it still names the state being freed, so the coop pump fails safe on every teardown path (a re-enter sets_battleStateto the live state first, so this never clobbers a valid pointer).DebriefingStatereadsgetSavedBattle()(notgetBattleState()), so the debrief path is unaffected.Verification
Isolated worktree build, TFTD data staged, headless 2-instance harness resuming the player's mid-battle save:
rc=3) / hangsSTR_ALIEN_COLONY_P2; hostcoopTurn=2(active), clientcoopTurn=3(waiting);battleInittrue on bothtest_coop_debrief_syncRepro / regression test
tools/coop_test/test_coop_nextstage_crash.py(+tftd_common.py, scrubbed fixturefixtures/tftd_base_assault.sav) resumes the save, kills the last stage-1 alien, drivesfinishBattle, and asserts both machines enter stage 2. Skips cleanly where TFTD data isn't staged (e.g. CI). The fixture is the player's own save with the coop player handles scrubbed toHostPlayer/ClientPlayer.🤖 Generated with Claude Code