Stop identity-manager deinit from dispatching onto its own queue

The previous fix routed forceSave() through queue.async(flags: .barrier),
and deinit called forceSave(). Dispatching onto the private concurrent
queue from deinit is a teardown hazard: the async block resurrects self
and enqueues barrier work that may not drain before process exit, so at
teardown swift-testing/libdispatch waits on that outstanding work and the
process never exits — the CI "Run Swift Tests (app)" job reached
[144/144], then wedged for ~5 minutes and was Killed:9. This surfaced now
because moving cryptographicIdentities into the persisted cache made far
more test-created managers persist on deinit.

Root cause confirmed locally: under LIBDISPATCH_COOPERATIVE_POOL_STRICT=1
(single-worker pool, mimicking a constrained CI runner) the old code
intermittently stalled the whole suite ~15s from deinit-scheduled barrier
work starving the pool; the fix is stable across 14 full coverage+parallel
runs with the process exiting ~3s after the last test.

Fix:
- deinit no longer touches `queue`. Persistence is already durable (every
  mutating API persists inline within its own barrier), so deinit only
  does a queue-free best-effort flush if `pendingSave` is still set — a
  direct read of in-hand state, safe because a deallocating object has no
  other live references mutating `cache`.
- forceSave() (lifecycle/app-termination only, never deinit) now uses a
  synchronous queue.sync(flags: .barrier): race-free `cache` read on the
  barrier context and nothing left scheduled at teardown. No re-entrant
  deadlock risk since it is never called from deinit.

Test: test_manyManagersDeinitDoNotWedgeTeardown churns 200 managers that
mutate then deinit and asserts the workload completes promptly; combined
with the existing concurrent race guard (still TSan-clean) this covers the
deinit path. Verified: swift test --parallel --enable-code-coverage green
14x with prompt process exit, and TSan-clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
jack
2026-07-02 00:54:02 +02:00
co-authored by Claude Fable 5
parent a55a16adcd
commit a8b741d605
3 changed files with 60 additions and 14 deletions
@@ -4,6 +4,33 @@ import BitFoundation
@testable import bitchat
final class SecureIdentityStateManagerTests: XCTestCase {
func test_manyManagersDeinitDoNotWedgeTeardown() {
// Teardown-hang guard: create many managers that mutate (scheduling
// fire-and-forget barrier saves) and then go out of scope, triggering
// deinit. A previous version dispatched forceSave() onto the private
// queue from deinit; at process teardown libdispatch/swift-testing
// waited on that outstanding barrier work and the process wedged
// (CI-only, killed at the watchdog). deinit must not schedule anything
// on `queue`. If this leaked scheduled work, a real CI run would hang;
// here we at least assert the whole workload completes promptly.
let iterations = 200
let start = Date()
for i in 0..<iterations {
autoreleasepool {
let manager = SecureIdentityStateManager(MockKeychain())
manager.upsertCryptographicIdentity(
fingerprint: Data(repeating: UInt8(i & 0xFF), count: 32).sha256Fingerprint(),
noisePublicKey: Data(repeating: UInt8(i & 0xFF), count: 32),
signingPublicKey: Data(repeating: UInt8((i + 1) & 0xFF), count: 32),
claimedNickname: "peer-\(i)"
)
// Drop `manager` -> deinit. Must not enqueue work on `queue`.
}
}
let elapsed = Date().timeIntervalSince(start)
XCTAssertLessThan(elapsed, 15, "manager churn/teardown took too long (possible deinit dispatch hang)")
}
func test_upsertCryptographicIdentity_withoutClaimedNicknameDoesNotCreateSocialIdentity() async {
let manager = SecureIdentityStateManager(MockKeychain())
let fingerprint = String(repeating: "aa", count: 32)