From 5780405dce58810f6f33dc98c907f12ab523fbfb Mon Sep 17 00:00:00 2001 From: jack <212554440+jackjackbits@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:21:49 +0100 Subject: [PATCH] Fix the SimulatedMesh announce-loss flake (#1564) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-authored-by: Claude Fable 5 --- bitchatTests/Simulation/SimulatedMesh.swift | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/bitchatTests/Simulation/SimulatedMesh.swift b/bitchatTests/Simulation/SimulatedMesh.swift index 1d5c5094..6caf379d 100644 --- a/bitchatTests/Simulation/SimulatedMesh.swift +++ b/bitchatTests/Simulation/SimulatedMesh.swift @@ -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()