From 1d0dc5822112c74a9ec30d8f31f84075956e57e5 Mon Sep 17 00:00:00 2001 From: jack <212554440+jackjackbits@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:21:46 +0100 Subject: [PATCH] Make the completion-grace restart test deterministic (#1563) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit immediateLegacyRestartDuringCompletionGrace injected a 0.03s initiator completion grace period and needed the restart initiation to arrive inside it. Constructing the restarted service (keypair generation) sits between starting that clock and processing the message, so on a starved CI runner the window expired first, the initiation was processed as a legitimate fresh handshake, and the nil-expectations cascaded — the most-sighted flake in CI (7 runs across #1502, #1477, #883, #1364, and main). The test now injects a grace period no test run can outlive, so the in-grace suppression and the duplicate-initiation coalescing are decided deterministically, and fires the deferred recovery through a DEBUG hook on NoiseSessionManager instead of waiting out the real timer. The hook cancels the scheduled work item before requesting recovery, so the converged-once assertion cannot double-fire either. Verified (count-checked via xcresulttool): the full 30-test NoiseEncryptionServiceTests suite green on the iOS simulator, and 30/30 x 5 consecutive runs under 16x CPU oversubscription (a 6th run was lost to a simulator app-launch refusal under load — no tests executed). The old test did not reproduce locally in 2 suite runs under the same load; the starvation needs the slow 2-core CI runner, so the diagnosis rests on the mechanism plus the identical assertion signature in all seven CI sightings. Co-authored-by: jack Co-authored-by: Claude Fable 5 --- bitchat/Noise/NoiseSessionManager.swift | 21 +++++++++++++++++++ bitchat/Services/NoiseEncryptionService.swift | 4 ++++ .../NoiseEncryptionServiceTests.swift | 10 +++++++-- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/bitchat/Noise/NoiseSessionManager.swift b/bitchat/Noise/NoiseSessionManager.swift index e8609aed..6e2b3c23 100644 --- a/bitchat/Noise/NoiseSessionManager.swift +++ b/bitchat/Noise/NoiseSessionManager.swift @@ -1028,6 +1028,27 @@ final class NoiseSessionManager { .cancel() } + #if DEBUG + /// Fires a pending suppressed-initiation recovery immediately instead of + /// waiting out the completion-grace timer, so tests can inject a grace + /// period too large to lose against a starved runner and still exercise + /// the recovery path deterministically. + func _test_fireSuppressedInitiationRecovery(for peerID: PeerID) { + managerQueue.sync(flags: .barrier) { + guard let pending = suppressedInitiationRecoveryTimeouts + .removeValue(forKey: peerID) else { + return + } + pending.cancel() + guard let current = sessions[peerID], + current.isEstablished() else { + return + } + requestHandshakeRecovery(for: peerID) + } + } + #endif + private func requestHandshakeRecovery( for peerID: PeerID, after delay: TimeInterval = 0 diff --git a/bitchat/Services/NoiseEncryptionService.swift b/bitchat/Services/NoiseEncryptionService.swift index 1bf2b574..5ee7608d 100644 --- a/bitchat/Services/NoiseEncryptionService.swift +++ b/bitchat/Services/NoiseEncryptionService.swift @@ -1089,6 +1089,10 @@ final class NoiseEncryptionService { func _test_initiateAutomaticRekey(for peerID: PeerID) throws { try initiateAutomaticRekey(for: peerID) } + + func _test_fireSuppressedInitiationRecovery(for peerID: PeerID) { + sessionManager._test_fireSuppressedInitiationRecovery(for: peerID) + } #endif deinit { diff --git a/bitchatTests/Services/NoiseEncryptionServiceTests.swift b/bitchatTests/Services/NoiseEncryptionServiceTests.swift index a2031983..c0cd293f 100644 --- a/bitchatTests/Services/NoiseEncryptionServiceTests.swift +++ b/bitchatTests/Services/NoiseEncryptionServiceTests.swift @@ -962,15 +962,20 @@ struct NoiseEncryptionServiceTests { @Test("Immediate legacy restart during completion grace converges once") func immediateLegacyRestartDuringCompletionGrace() async throws { + // The grace period must still be open when the restart initiation + // arrives below. A small value races the wall clock on a starved + // runner, so inject one no test run can outlive; the recovery half + // is then fired explicitly instead of waiting out the timer. + let unlosableGracePeriod: TimeInterval = 600 let firstKeychain = MockKeychain() let secondKeychain = MockKeychain() let first = NoiseEncryptionService( keychain: firstKeychain, - recentInitiatorCompletionGracePeriod: 0.03 + recentInitiatorCompletionGracePeriod: unlosableGracePeriod ) let second = NoiseEncryptionService( keychain: secondKeychain, - recentInitiatorCompletionGracePeriod: 0.03 + recentInitiatorCompletionGracePeriod: unlosableGracePeriod ) let firstPeerID = PeerID(publicKey: first.getStaticPublicKeyData()) let secondPeerID = PeerID(publicKey: second.getStaticPublicKeyData()) @@ -1035,6 +1040,7 @@ struct NoiseEncryptionServiceTests { ) #expect(lower.hasEstablishedSession(with: higherPeerID)) + lower._test_fireSuppressedInitiationRecovery(for: higherPeerID) let requested = await TestHelpers.waitUntil( { recovery.messages.count == 1 }, timeout: TestConstants.longTimeout