Fix the SimulatedMesh announce-loss flake (#1564)

SimulatedMesh.addNode installed the outbound tap one statement after
setNickname, but setNickname force-announces asynchronously on the
engine. When a starved runner let that slot run inside the gap, the
announce was emitted invisibly while still stamping the wall-clock
announce throttle, and announceAll's forced announce — arriving well
inside the 0.15s forced minimum interval — was swallowed. No discovery
traffic ever reached the mesh, so bindings stayed nil and peer lists
empty: the exact 4-issue signature that failed three main runs and one
PR run on July 30.

Reproduced deterministically by forcing the ordering with a 5ms sleep
after setNickname: all 8 SimulatedMesh tests fail on the old harness
and pass on the fixed one.

Fixes: install the tap before setNickname so an early nickname announce
is captured instead of lost; reset each node's throttle in announceAll
so wall-clock throttle debt can never swallow the discovery round
(forceAnnounce(from:) deliberately keeps no-reset — the panic-rotation
tests pin the production reset behavior through it); and take the lock
around addNode's array appends, which could race the tap reading
`emitted` on an earlier node's engine.

Verified: suite green normally, 8/8 tests x 6 runs under 16x CPU
oversubscription, and 8/8 under the adversarial forced ordering —
all count-verified via xcresulttool (an earlier single-test
-only-testing filter silently matched zero tests, so every result
here was re-checked against reported test counts).

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
jack 2026-07-30 21:21:49 +01:00 committed by GitHub
parent 1d0dc58221
commit 5780405dce
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -58,10 +58,19 @@ final class SimulatedMesh {
)
let index = nodes.count
let node = Node(service: service, scheduler: scheduler)
// An earlier node's engine can fire its tap (which reads `emitted`
// under the lock) while this append reallocates the array.
lock.lock()
nodes.append(node)
neighbors.append([])
emitted.append([])
service.setNickname(nickname)
lock.unlock()
// The tap must be live before `setNickname` below: setNickname
// force-announces asynchronously on the engine, and if that slot
// ran in the gap before a later tap install, the announce was
// emitted invisibly while still stamping the wall-clock announce
// throttle swallowing `announceAll`'s forced announce on a
// starved runner (the CI flake this ordering fixes).
service._test_onOutboundPacket = { [weak self] packet in
// Runs on the sender's engine; only buffer here delivering
// inline would nest one engine inside another.
@ -71,6 +80,7 @@ final class SimulatedMesh {
self.emitted[index].append(packet)
self.lock.unlock()
}
service.setNickname(nickname)
return node
}
@ -175,8 +185,16 @@ final class SimulatedMesh {
}
/// Full discovery round: every node announces, traffic settles.
///
/// Resets each node's announce throttle first: the throttle window is
/// wall-clock, so any announce that already ran (setNickname's, in
/// `addNode`) would otherwise swallow this forced one whenever the two
/// land within the forced minimum interval which is always, on any
/// runner. `forceAnnounce(from:)` deliberately does NOT reset the
/// panic-rotation tests pin the production reset behavior through it.
func announceAll() {
for node in nodes {
node.service._test_resetAnnounceThrottle()
node.service._test_forceAnnounce()
}
pump()