mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-24 23:45:18 +00:00
Fix deadlock in identity-cache forceSave (CI hang)
The forceSave() rewrite used queue.sync(flags: .barrier), but forceSave is also called from deinit. The debounce timer's barrier hop captured self strongly, so when that block dropped the last reference the manager deallocated *on* the identity queue — deinit -> forceSave -> queue.sync then deadlocked synchronizing onto the queue it was already running on. This hung the test process at exit (CI SIGKILL / exit 137). - forceSave() now detects (via a queue-specific key) when it is already executing on the queue and runs the save directly instead of sync-ing onto itself. - The timer's barrier hop now captures self weakly, so it can no longer trigger a deallocation on the queue in the first place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -150,6 +150,10 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol {
|
|||||||
|
|
||||||
// Thread safety
|
// Thread safety
|
||||||
private let queue = DispatchQueue(label: "bitchat.identity.state", attributes: .concurrent)
|
private let queue = DispatchQueue(label: "bitchat.identity.state", attributes: .concurrent)
|
||||||
|
/// Marks `queue` so `forceSave()` can detect when it is already executing on
|
||||||
|
/// it (e.g. when `deinit` is triggered from inside a queue block) and run
|
||||||
|
/// directly instead of `queue.sync`-ing onto itself, which would deadlock.
|
||||||
|
private static let queueSpecificKey = DispatchSpecificKey<UInt8>()
|
||||||
|
|
||||||
// Debouncing for keychain saves.
|
// Debouncing for keychain saves.
|
||||||
// A DispatchSourceTimer on `queue` is used rather than Timer.scheduledTimer:
|
// A DispatchSourceTimer on `queue` is used rather than Timer.scheduledTimer:
|
||||||
@@ -170,7 +174,8 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol {
|
|||||||
|
|
||||||
init(_ keychain: KeychainManagerProtocol) {
|
init(_ keychain: KeychainManagerProtocol) {
|
||||||
self.keychain = keychain
|
self.keychain = keychain
|
||||||
|
queue.setSpecific(key: Self.queueSpecificKey, value: 1)
|
||||||
|
|
||||||
// Retrieve (or, only on genuine first run, generate) the cache
|
// Retrieve (or, only on genuine first run, generate) the cache
|
||||||
// encryption key. We MUST distinguish "key doesn't exist yet" from a
|
// encryption key. We MUST distinguish "key doesn't exist yet" from a
|
||||||
// transient failure (device locked / access denied): the legacy
|
// transient failure (device locked / access denied): the legacy
|
||||||
@@ -253,9 +258,11 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol {
|
|||||||
let timer = DispatchSource.makeTimerSource(queue: queue)
|
let timer = DispatchSource.makeTimerSource(queue: queue)
|
||||||
timer.schedule(deadline: .now() + saveDebounceInterval)
|
timer.schedule(deadline: .now() + saveDebounceInterval)
|
||||||
timer.setEventHandler { [weak self] in
|
timer.setEventHandler { [weak self] in
|
||||||
guard let self else { return }
|
|
||||||
// Hop to a barrier so performSave reads/writes state exclusively.
|
// Hop to a barrier so performSave reads/writes state exclusively.
|
||||||
self.queue.async(flags: .barrier) { self.performSave() }
|
// Capture self weakly here too: a strong capture would let the object
|
||||||
|
// deallocate on `queue` when this block drops the last reference,
|
||||||
|
// sending deinit -> forceSave through a sync-on-self deadlock.
|
||||||
|
self?.queue.async(flags: .barrier) { [weak self] in self?.performSave() }
|
||||||
}
|
}
|
||||||
saveTimer = timer
|
saveTimer = timer
|
||||||
timer.resume()
|
timer.resume()
|
||||||
@@ -289,11 +296,18 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol {
|
|||||||
// Force immediate save (for app termination). Runs synchronously on `queue`
|
// Force immediate save (for app termination). Runs synchronously on `queue`
|
||||||
// so the write completes before the caller proceeds (e.g. app exit).
|
// so the write completes before the caller proceeds (e.g. app exit).
|
||||||
func forceSave() {
|
func forceSave() {
|
||||||
queue.sync(flags: .barrier) {
|
let work = {
|
||||||
self.saveTimer?.cancel()
|
self.saveTimer?.cancel()
|
||||||
self.saveTimer = nil
|
self.saveTimer = nil
|
||||||
self.performSave()
|
self.performSave()
|
||||||
}
|
}
|
||||||
|
// If we are already on `queue` (e.g. deinit fired from inside a queue
|
||||||
|
// block), run directly — `queue.sync` onto the current queue deadlocks.
|
||||||
|
if DispatchQueue.getSpecific(key: Self.queueSpecificKey) != nil {
|
||||||
|
work()
|
||||||
|
} else {
|
||||||
|
queue.sync(flags: .barrier, execute: work)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Social Identity Management
|
// MARK: - Social Identity Management
|
||||||
|
|||||||
Reference in New Issue
Block a user