You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
ServiceQueue.background has two overloads. The synchronous one really does serialize:
service.queue.async{letres=tryblocking() // runs on the queue thread
continuation.resume(...)}
The async one does not:
service.queue.async{Task{ // hops straight off the DispatchQueue
letresult=tryawaitexecute()...}}
The Task detaches immediately, so the queue block returns before any work happens and the body runs on the cooperative pool. Overload resolution is by closure shape, so adding a single await inside a ServiceQueue.background block silently converts it from serialized to unserialized — no diagnostic, no visible change at the call site.
Found while reviewing #648, where replaceHwSnapshot's read → delete → upsert sequence had picked up the async overload this way and was racing markOnchainActivityAsTransfer. Fixed there by keeping the closure synchronous; the general trap is still armed.
Confirmed scope
Not affected — LDK. All 41 ServiceQueue.background(.ldk) call sites pass synchronous closures and are genuinely serialized on ldkQueue. Nothing on the Lightning path depends on the async overload's current behaviour.
Two read-modify-write cases on .core (both pre-existing, neither on a hardware-wallet path, both a ~3-line fix — move refreshBoostTxIdsCache after the await):
ActivityService.delete(id:walletId:) — Bitkit/Services/CoreService.swift:1385 (no production caller; only BitkitTests/ActivityListTest.swift)
While these stand, markOnchainActivityAsTransfer's documented "single core-queue transaction so a concurrent watcher sync can't clobber it" is only partly true.
The larger finding.OnChainHwService (8 call sites) and TrezorService (15) both carry the class comment:
All operations run on ServiceQueue.background(.core) to ensure thread safety
Every one of those call sites uses the async overload, so none of them are actually serialized and the documented guarantee does not hold. Unlike the two above, these genuinely await async FFI (Electrum queries, device I/O), so the synchronous overload is not the fix — blocking a queue thread on network I/O would be worse.
Suggested fix
Fix the two .core read-modify-write cases by moving the awaited work outside the queue block.
Give the async path real serialization — an actor per service domain, so await-ing work can queue without blocking a thread — and either adopt it in OnChainHwService/TrezorService or correct their class comments to state what they actually guarantee.
Rename the async overload (e.g. ServiceQueue.backgroundAsync) so the two can never be selected by inference again. This is the part that stops the bug recurring; it touches ~40 call sites across CoreService/LightningService/BackupService, which is why it was kept out of feat: persist hardware wallet activities #648.
Problem
ServiceQueue.backgroundhas two overloads. The synchronous one really does serialize:The async one does not:
The
Taskdetaches immediately, so the queue block returns before any work happens and the body runs on the cooperative pool. Overload resolution is by closure shape, so adding a singleawaitinside aServiceQueue.backgroundblock silently converts it from serialized to unserialized — no diagnostic, no visible change at the call site.Found while reviewing #648, where
replaceHwSnapshot's read → delete → upsert sequence had picked up the async overload this way and was racingmarkOnchainActivityAsTransfer. Fixed there by keeping the closure synchronous; the general trap is still armed.Confirmed scope
Not affected — LDK. All 41
ServiceQueue.background(.ldk)call sites pass synchronous closures and are genuinely serialized onldkQueue. Nothing on the Lightning path depends on the async overload's current behaviour.Two read-modify-write cases on
.core(both pre-existing, neither on a hardware-wallet path, both a ~3-line fix — moverefreshBoostTxIdsCacheafter theawait):ActivityService.upsertList—Bitkit/Services/CoreService.swift:380(callers:BackupServicerestore,MigrationsService)ActivityService.delete(id:walletId:)—Bitkit/Services/CoreService.swift:1385(no production caller; onlyBitkitTests/ActivityListTest.swift)While these stand,
markOnchainActivityAsTransfer's documented "single core-queue transaction so a concurrent watcher sync can't clobber it" is only partly true.The larger finding.
OnChainHwService(8 call sites) andTrezorService(15) both carry the class comment:Every one of those call sites uses the async overload, so none of them are actually serialized and the documented guarantee does not hold. Unlike the two above, these genuinely await async FFI (Electrum queries, device I/O), so the synchronous overload is not the fix — blocking a queue thread on network I/O would be worse.
Suggested fix
.coreread-modify-write cases by moving the awaited work outside the queue block.await-ing work can queue without blocking a thread — and either adopt it inOnChainHwService/TrezorServiceor correct their class comments to state what they actually guarantee.ServiceQueue.backgroundAsync) so the two can never be selected by inference again. This is the part that stops the bug recurring; it touches ~40 call sites acrossCoreService/LightningService/BackupService, which is why it was kept out of feat: persist hardware wallet activities #648.References
ActivityService.replaceHwSnapshotand the comment on its queue block.Bitkit/Services/ServiceQueue.swift— the two overloads.