>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #3591953. --> Reported by: [mably](https://www.drupal.org/user/3375160) Related to !12 >>> <h2>Summary</h2> <p>In <code>ChainArchiver::createBareSegment()</code>, the acknowledgment-bisection guard <code>assertNoAcknowledgmentBoundaryConflict()</code> runs BEFORE the chain-write lock is acquired. A concurrent <code>AuditTrailVerifier::acknowledgeChainReset()</code> call (which holds no chain-write lock) can land an INSERT into <code>audit_trail_acknowledgment</code> in the window between the guard check and the bare segment INSERT, producing a bare segment whose <code>[from_id, to_id]</code> range bisects the just-inserted acknowledgment -- the exact invariant the guard exists to prevent.</p> <h2>Reproduction (race trace)</h2> <p>Two threads on the same chain, no shared lock between them:</p> <ol> <li>Thread A: <code>createBareSegment(chain, F, T)</code>.<br> <ol type="a"> <li>Line 1078: <code>assertNoAcknowledgmentBoundaryConflict(chain, F, T)</code> -- scans <code>audit_trail_acknowledgment</code>, finds no bisecting ack, returns clean.</li> <li>Line 1080: <code>acquireChainWriteLock(chain)</code> -- contended, blocks briefly.</li> </ol> </li> <li>Thread B: <code>acknowledgeChainReset(chain, Fa, Ta)</code> where the new ack range would bisect <code>[F, T]</code>.<br> <ol type="a"> <li>Validates range bounds.</li> <li>Resolves anchors, signs HMAC.</li> <li>INSERTs the ack -- takes no chain-write lock; nothing blocks it.</li> </ol> </li> <li>Thread A unblocks on the lock, proceeds:<br> <ol type="a"> <li>Reads <code>head_id</code> (unchanged).</li> <li>Resolves anchors, signs HMAC.</li> <li>INSERTs the bare segment <code>[F, T]</code>.</li> </ol> </li> </ol> <p>Post-race state: a bare segment exists whose range bisects an existing acknowledgment, which is the exact corruption shape the guard is supposed to prevent. Downstream archive of the bare would unarchive a partial ack range or split an ack across two NDJSON files; either way the verifier flags the bisection.</p> <h2>Root cause</h2> <p>Two correctness conditions are checked OUTSIDE the chain-write lock:</p> <ul> <li><code>src/Archive/ChainArchiver.php:1078</code> -- <code>assertNoAcknowledgmentBoundaryConflict</code> in <code>createBareSegment</code>, runs before line 1080's <code>acquireChainWriteLock</code>.</li> <li><code>src/AuditTrailVerifier.php:1602</code> -- <code>acknowledgeChainReset</code> takes no chain-write lock at all; only validates + INSERTs the ack row directly.</li> </ul> <p>Both paths can therefore mutate or read <code>audit_trail_acknowledgment</code> without serializing against each other.</p> <p>Pre-existing in the codebase; not introduced by MR !11 (#3591949), which surfaced it during the lock-handling audit. <code>createBareSegment</code> is reached from two production call sites: <code>ensureSegmentCoverage()</code> (cron + drush + snap path) and <code>AuditTrailSegmentAddForm</code> (admin "add segment" form). Both are exposed to the race.</p> <h2>Severity</h2> <p>Low in practice, real in principle:</p> <ul> <li>Race window is microseconds wide (one SELECT between the guard scan and the lock acquire).</li> <li>Both call sites are slow human-driven operations (cron tick, admin form submit, drush command) -- collisions in production are near-zero.</li> <li>Detection: post-corruption, <code>verifyChain()</code> flags the bisecting bare via the segment cross-reference + ack walks; the operator sees an integrity break with a clear "ack range bisected" diagnostic. No silent data loss.</li> <li>Recovery: operator splits the ack, drops the bare, or merges via the admin form.</li> </ul> <p>Tracking as a separate bug rather than rolling into another MR because the fix touches both <code>ChainArchiver</code> and <code>AuditTrailVerifier</code> and deserves its own test.</p> <h2>Suggested fix</h2> <p>Move <code>assertNoAcknowledgmentBoundaryConflict</code> INSIDE the chain-write lock in <code>createBareSegment</code>:</p> <pre><pre>public function createBareSegment(string $chain_id, int $from_id, int $to_id): int {<br>&nbsp; // ... range validation stays outside the lock (no DB access) ...<br><br>&nbsp; $lock_name = $this-&amp;gt;acquireChainWriteLock($chain_id, 'declare a segment');<br>&nbsp; try {<br>&nbsp;&nbsp;&nbsp; // Move the ack guard INSIDE the lock so a concurrent<br>&nbsp;&nbsp;&nbsp; // acknowledgeChainReset() cannot slip a bisecting ack in<br>&nbsp;&nbsp;&nbsp; // between the guard scan and the bare INSERT.<br>&nbsp;&nbsp;&nbsp; $this-&amp;gt;assertNoAcknowledgmentBoundaryConflict($chain_id, $from_id, $to_id);<br>&nbsp;&nbsp;&nbsp; // ... rest unchanged ...<br>&nbsp; }<br>&nbsp; finally {<br>&nbsp;&nbsp;&nbsp; $this-&amp;gt;lock-&amp;gt;release($lock_name);<br>&nbsp; }<br>}</pre></pre><p>Symmetric fix on the ack-emitting side: <code>acknowledgeChainReset</code> should also acquire the chain-write lock for validation + INSERT, so it serializes against <code>createBareSegment</code> the same way. Alternative: only acquire the lock for the INSERT itself and re-validate anchors inside. The simpler "lock the whole call" is fine -- ack writes are rare.</p> <h2>Tests to add</h2> <ul> <li>Race-window test: stub the chain-write lock backend to block <code>createBareSegment</code> at the acquire step, INSERT a bisecting ack on a separate connection (or fake the DB state directly), release the lock, assert <code>createBareSegment</code> throws with the existing "would be bisected" error instead of silently INSERTing the corrupted bare. The point is to exercise the now-INSIDE-the-lock guard against a state-change that happened DURING the lock wait.</li> <li>Sibling test for <code>acknowledgeChainReset</code> taking the lock: pre-create a bare at <code>[F, T]</code> using the chain-write lock, then attempt <code>acknowledgeChainReset</code> with a bisecting range from a parallel context -- should be serialized + refused once the lock is held by ack.</li> </ul> <p>Both tests will fail without the fix; verified per the new "tests fail without the fix" convention (#3591949).</p>