Skip to content

Commit 1f0a3e0

Browse files
Blackvipe99claude
andauthored
Fix AI eval thread races (timeout cancellation, parallel mustAttack Combat mutation) (#11161)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 5bac6e0 commit 1f0a3e0

2 files changed

Lines changed: 36 additions & 11 deletions

File tree

forge-ai/src/main/java/forge/ai/AiAttackController.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,12 @@ && isEffectiveAttacker(ai, attacker, combat, finalDefender)) {
940940
}
941941
}
942942
if (mustAttackDef != null) {
943-
combat.addAttacker(attacker, mustAttackDef);
943+
// combat is shared across these parallel futures and its attacker
944+
// multimap is not thread-safe; unsynchronized addAttacker calls
945+
// collide (ConcurrentModificationException, dropped attackers)
946+
synchronized (combat) {
947+
combat.addAttacker(attacker, mustAttackDef);
948+
}
944949
attackersLeft.remove(attacker);
945950
numForcedAttackers.incrementAndGet();
946951
}

forge-ai/src/main/java/forge/ai/AiController.java

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public class AiController {
9898
private int lastAttackAggression;
9999
private boolean useLivingEnd;
100100
private List<SpellAbility> skipped;
101-
private boolean timeoutReached;
101+
private volatile boolean timeoutReached;
102102

103103
public AiController(final Player computerPlayer, final Game game0) {
104104
player = computerPlayer;
@@ -1601,7 +1601,7 @@ private SpellAbility chooseSpellAbilityToPlayFromList(final List<SpellAbility> a
16011601
continue;
16021602
}
16031603

1604-
if (timeoutReached) {
1604+
if (timeoutReached || Thread.currentThread().isInterrupted()) {
16051605
timeoutReached = false;
16061606
break;
16071607
}
@@ -1681,16 +1681,36 @@ else if (!sa.getHostCard().isPermanent() && sa.canCastTiming(player)
16811681
try {
16821682
return future.get(game.getAITimeout(), TimeUnit.SECONDS);
16831683
} catch (InterruptedException | ExecutionException | TimeoutException e) {
1684+
e.printStackTrace();
1685+
if (e instanceof TimeoutException) {
1686+
// log where the eval thread currently is - each timeout doubles as a
1687+
// profiler sample for diagnosing remaining AI slowdowns from user logs
1688+
StringBuilder sb = new StringBuilder("AI eval thread at timeout:");
1689+
StackTraceElement[] evalStack = t.getStackTrace();
1690+
for (int i = 0; i < Math.min(30, evalStack.length); i++) {
1691+
sb.append("\n\tat ").append(evalStack[i]);
1692+
}
1693+
System.out.println(sb);
1694+
}
1695+
// ask the eval thread to exit at the next SpellAbility check first: a brutal
1696+
// Thread.stop() mid-evaluation can leave partially mutated shared state behind
1697+
timeoutReached = true;
1698+
future.cancel(true);
16841699
try {
1685-
e.printStackTrace();
1686-
t.stop();
1687-
} catch (UnsupportedOperationException | NoSuchMethodError ex) {
1688-
// Stop support: dropped by Android and Java 20 / 26 removed it completely - so sadly thread will keep running
1689-
timeoutReached = true;
1690-
future.cancel(true);
1691-
// TODO wait a few more seconds to try and exit at a safe point before letting the engine continue
1692-
// TODO mark some as skipped to increase chance to find something playable next priority
1700+
t.join(500);
1701+
} catch (InterruptedException ie) {
1702+
Thread.currentThread().interrupt();
1703+
}
1704+
if (t.isAlive()) {
1705+
// last resort, see #8302: the eval thread may be stuck inside a single
1706+
// evaluation or an infinite loop and never reach the cooperative exit
1707+
try {
1708+
t.stop();
1709+
} catch (UnsupportedOperationException | NoSuchMethodError ex) {
1710+
// Stop support: dropped by Android and Java 20 / 26 removed it completely - so sadly thread will keep running
1711+
}
16931712
}
1713+
// TODO mark some as skipped to increase chance to find something playable next priority
16941714
return null;
16951715
}
16961716
}

0 commit comments

Comments
 (0)