Summary
AttackExecution.handleDeadDefender() iterates directly over the live target.tiles() collection while simultaneously calling this._owner.conquer(tile) inside that same loop. conquer() mutates the underlying Set<TileRef> mid-iteration, triggering JavaScript's iterator invalidation behavior — causing silent data corruption followed by a delayed, hard-to-trace server crash.
Root Cause
In src/core/execution/AttackExecution.ts, the upstream cleanup loop looks like this:
// ⚠️ Upstream (unpatched) — iterates live collection while mutating it
for (const tile of target.tiles()) {
this._owner.conquer(tile); // ← mutates the same Set mid-iteration
}
JavaScript's Set iterator specification (ECMA-262 §23.2.5.2.1) guarantees that:
- Entries deleted before being visited by the iterator will be skipped silently — no exception is thrown.
- Entries added after iteration begins will be visited — potentially extending the loop.
This means conquer() calls that transfer or remove tiles from the Set cause the iterator to silently skip remaining defender tiles, leaving them in a corrupt intermediate state.
Failure Mode: Two-Stage Corruption + Crash
Stage 1 — Ghost Territory (Silent Corruption)
The iterator skips a subset of the defender's tiles unpredictably. Those tiles remain registered under the dead/eliminated Player object in the game state, but that player has zero tiles from the game's perspective. The result is orphaned phantom territories — tiles with an owner that no longer exists as an active participant.
Stage 2 — Downstream Null-Dereference Crash (Delayed)
On the next game tick, downstream systems process every tile in the map — including the orphaned ones. Systems such as win condition checks, score calculation, and border updates call this.mg.owner(tile) on a ghost tile and receive a stale Player reference — or null. Attempting to call methods on this reference (e.g., .numTilesOwned(), .id()) throws an uncaught TypeError, crashing the entire server process.
Because the crash originates several ticks after the corrupting event in AttackExecution.ts, stack traces point at the wrong system — making this extremely difficult to diagnose without understanding the iterator invalidation root cause.
Why This Is a Verified Structural Vulnerability
This is not a speculative bug — it is structurally guaranteed by the ECMA-262 specification. The behavior is deterministic: any game where a defender's tile count drops below 100 and triggers handleDeadDefender() is at risk, regardless of map or player count.
Proposed Fix
Snapshot the tile collection into a static array before beginning iteration:
// ✅ Fixed — snapshot prevents iterator invalidation
for (let i = 0; i < 100; i++) {
const remainingTiles = Array.from(target.tiles()); // frozen copy
if (remainingTiles.length === 0) break;
for (const tile of remainingTiles) {
this._owner.conquer(tile); // safe — not touching the iterator
}
}
Array.from(target.tiles()) creates a complete, isolated snapshot of the Set at that moment. All subsequent conquer() mutations operate on the live Set but the iterator is unaffected, eliminating both the ghost territory and the downstream crash.
Affected File
src/core/execution/AttackExecution.ts — handleDeadDefender() method
Linked PR
#3943
Summary
AttackExecution.handleDeadDefender()iterates directly over the livetarget.tiles()collection while simultaneously callingthis._owner.conquer(tile)inside that same loop.conquer()mutates the underlyingSet<TileRef>mid-iteration, triggering JavaScript's iterator invalidation behavior — causing silent data corruption followed by a delayed, hard-to-trace server crash.Root Cause
In
src/core/execution/AttackExecution.ts, the upstream cleanup loop looks like this:JavaScript's
Setiterator specification (ECMA-262 §23.2.5.2.1) guarantees that:This means
conquer()calls that transfer or remove tiles from theSetcause the iterator to silently skip remaining defender tiles, leaving them in a corrupt intermediate state.Failure Mode: Two-Stage Corruption + Crash
Stage 1 — Ghost Territory (Silent Corruption)
The iterator skips a subset of the defender's tiles unpredictably. Those tiles remain registered under the dead/eliminated
Playerobject in the game state, but that player has zero tiles from the game's perspective. The result is orphaned phantom territories — tiles with an owner that no longer exists as an active participant.Stage 2 — Downstream Null-Dereference Crash (Delayed)
On the next game tick, downstream systems process every tile in the map — including the orphaned ones. Systems such as win condition checks, score calculation, and border updates call
this.mg.owner(tile)on a ghost tile and receive a stalePlayerreference — ornull. Attempting to call methods on this reference (e.g.,.numTilesOwned(),.id()) throws an uncaughtTypeError, crashing the entire server process.Because the crash originates several ticks after the corrupting event in
AttackExecution.ts, stack traces point at the wrong system — making this extremely difficult to diagnose without understanding the iterator invalidation root cause.Why This Is a Verified Structural Vulnerability
This is not a speculative bug — it is structurally guaranteed by the ECMA-262 specification. The behavior is deterministic: any game where a defender's tile count drops below 100 and triggers
handleDeadDefender()is at risk, regardless of map or player count.Proposed Fix
Snapshot the tile collection into a static array before beginning iteration:
Array.from(target.tiles())creates a complete, isolated snapshot of theSetat that moment. All subsequentconquer()mutations operate on the liveSetbut the iterator is unaffected, eliminating both the ghost territory and the downstream crash.Affected File
src/core/execution/AttackExecution.ts—handleDeadDefender()methodLinked PR
#3943