Make the completion-grace restart test deterministic (#1563)

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 <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:46 +01:00 committed by GitHub
parent 6414a59851
commit 1d0dc58221
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 33 additions and 2 deletions

View File

@ -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

View File

@ -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 {

View File

@ -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