diff options
| author | Scott Gasch <[email protected]> | 2026-10-14 11:44:16 -0700 |
|---|---|---|
| committer | Scott Gasch <[email protected]> | 2026-10-14 11:44:16 -0700 |
| commit | 91f01c607338eabb10ae1f8c144be55071d174a5 (patch) | |
| tree | 89d14a9f6f6533152cad409095487f556ba3087f | |
| parent | b3d4d0781faa189c74b05fec96814e3f30e449f9 (diff) | |
Fix WouldGiveCheck race in StartParallelSearch's move snapshot
StartParallelSearch's snapshot loop (split.c, the move list handed to
helper threads at a split) skipped recomputing WouldGiveCheck for
v == uMoveNum on the assumption that Search()'s own loop had already
computed it moments earlier when it originally selected that move.
That assumption only holds if re-running SelectBestWithHistory(ctx,
uMoveNum) is a no-op. It isn't: the loop calls SelectBestWithHistory
for every v including uMoveNum, which rescans [v, uEnd) using
g_HistoryCounters/g_ContinuationHistory -- process-global state
written by every other searcher thread with no lock. Between
Search()'s original selection and this second one a few lines later,
another thread can mutate those tables enough to change which move
wins the scan, swapping a *different* move into slot uMoveNum than
the one originally selected -- one whose check bit was never
computed, inheriting a stale/garbage value instead.
That move, with its wrong check bit, got handed to a helper thread via
g_SplitInfo[u].mvf[], tripping HelpSearch's own
IS_CHECKING_MOVE/InCheck invariant (visible as "Assertion failure in
split.c, line 1143" under DEBUG) or, in a release build, corrupting
move generation further down the line (observed as "Typhoon PANIC 0x4
... Location: generate.c" via InvalidGenerator, when a move wrongly
believed non-checking reached a dispatch table expecting the opposite
invariant).
Confirmed via a DEBUG=1 build at --cpus 4: reliably reproduced within
the first ~15 positions of ecm_confident_quick at sd10 before this fix
(same assert, different positions across repeated runs -- consistent
with a real race, not a deterministic bug), and ran 43 consecutive
positions with zero assertion failures after. Also confirmed
pre-existing at HEAD~2 (3d54c2c, before this session's prior two
commits), so unrelated to the search-unification work in progress.
Fix: always recompute WouldGiveCheck for every move in this loop,
including uMoveNum. This loop runs once per split (not once per move
tried), so the extra call is split overhead, not a per-node cost.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
| -rw-r--r-- | src/split.c | 31 |
1 files changed, 26 insertions, 5 deletions
diff --git a/src/split.c b/src/split.c index e72bd2c..2997661 100644 --- a/src/split.c +++ b/src/split.c @@ -648,12 +648,33 @@ Return value: // unconditionally now too -- see its retired // NumLeftoverMovesToSelect budget comment). // + // Recompute WouldGiveCheck unconditionally, even for + // v == uMoveNum (whose check bit Search()'s own loop + // already set moments ago, before deciding to split). + // g_HistoryCounters/g_ContinuationHistory -- which + // SelectBestWithHistory's scan above just read to rank + // candidates -- are process-global and written by + // every other searcher thread with no lock. Between + // that earlier selection and this one, another thread + // can mutate them enough to change which move wins + // the [uMoveNum, uEnd) scan, swapping a DIFFERENT move + // into slot uMoveNum than the one Search() originally + // selected -- one whose check bit was never computed. + // The stale/garbage bit that move inherited from + // whatever slot it came from was then handed to a + // helper thread via g_SplitInfo[u].mvf[], tripping + // HelpSearch's own IS_CHECKING_MOVE/InCheck ASSERT (or, + // in a release build, corrupting move generation + // further down the line) -- confirmed nondeterministic + // across runs and reproducible pre-dating this fix, at + // --cpus 4 only (needs real concurrent writers to the + // history tables). The extra WouldGiveCheck call this + // costs is now just part of split overhead, not a + // per-node cost -- this loop only ever runs once per + // split, not once per move tried. SelectBestWithHistory(ctx, v); - if (v != uMoveNum) // First one has WouldGiveCheck already computed - { - ctx->sMoveStack.mvf[v].mv.bvFlags |= - WouldGiveCheck(ctx, ctx->sMoveStack.mvf[v].mv); - } + ctx->sMoveStack.mvf[v].mv.bvFlags |= + WouldGiveCheck(ctx, ctx->sMoveStack.mvf[v].mv); ASSERT(!(ctx->sMoveStack.mvf[v].bvFlags & MVF_MOVE_SEARCHED)); g_SplitInfo[u].mvf[g_SplitInfo[u].uRemainingMoves] = ctx->sMoveStack.mvf[v]; |
