Run connection state as atomic blocks on one queue, and serialise reassembly

[kmatzen]

Last of four stacked PRs. Base is fix/sleep-visibility (#89), not main. This is the largest and least verified — see review notes.

The problem

BLEManager keeps connection state in three @Published dictionaries, read from the CoreBluetooth delegate queue and mutated on main. Swift Dictionary is not thread-safe, so that is undefined behaviour, not a stale read — and a decision made from a value read on one queue can be invalidated before the write lands on another.

I modelled the candidate disciplines before writing anything (specs/ConnectionConcurrency.tla):

Discipline	No data race	No lost update
current — read on BLE queue, write dispatched to main	✗	✗
ensure every write is on main	✗	✗
whole read-decide-write in one main block	✓	✓
whole operation on one serial queue / actor	✓	✓

Dispatching only the writes to main fixes nothing. That is worth stating plainly because it is the shape several sites already had, and because a diff that wraps a mutation in DispatchQueue.main.async looks like a fix and is not one.

Sites corrected

Each of these split the read from the write across a queue hop:

• handleConnectionSuccess looked up connectingGoPros on the CoreBluetooth queue and inserted that reference into connectedGoPros in a later main block — the camera could be removed in between and the stale reference reinserted.
• didDiscover checked "already connected?" on the CoreBluetooth queue and acted on it in a dispatched block.
• powerDownGoPro looked up the camera on the caller's queue and dispatched only the removal.
• setDateTime, the WiFi response callbacks, and the device-query timer all read state off the state queue.

Two reads were simply unnecessary and are gone: handleCommandResponse looked up a camera for a display name already in scope, and createAppStateContext built crash-report context from the CoreBluetooth queue.

Ordering between blocks

onStateQueue always dispatches rather than running inline when already on main. I wrote the inline version first; a second model (specs/StateQueueOrdering.tla) inverts two operations in three steps — work dispatched earlier from the CoreBluetooth queue is still waiting while a later main-thread caller executes immediately.

Atomicity of each block does not imply a consistent order between blocks. The cost is that main-thread callers are deferred by a runloop turn; I checked all nine call sites and none depend on the effect landing synchronously.

File split

BLEManager.swift is at SwiftLint's 2600-line ceiling. The state-queue primitives move to BLEManager+StateQueue.swift, which is where they belong anyway — they are infrastructure, not connection logic. The file lands at 2556.

Reassembler

Same hazard, different queues: processPacket runs on the CoreBluetooth queue while checkTimeouts runs on the device-query timer's queue. Its state does not drive SwiftUI, so it gets a dedicated serial queue rather than main, keeping packet handling off the main thread.

This one is verified empirically — 4,000 concurrent iterations under ThreadSanitizer:

serialised:               0 races
control (queue removed):  WARNING: ThreadSanitizer: Swift access race
                          in BLEPacketReconstructor.handleStartPacket

The control matters: it proves TSan was actually detecting, so the clean run means something.

Review notes — please read before merging

BLEManager's discipline rests on the model plus the debug assertions and nothing else. Exercising it needs real BLE traffic, so the largest part of this change has no empirical verification. The 135 passing tests (verified via xcodebuild test on iOS 26.5) are all pure logic and would stay green whether or not the queue discipline is right; they confirm nothing was broken, not that the concurrency is correct.

There were ~70 original access sites. I would assume I missed some.

Suggested validation: run with ThreadSanitizer enabled against a real camera, connect and disconnect a few times, and watch for both TSan reports and assertOnStateQueue failures. The assertions will find missed paths faster than TSan will.

If you want to merge the first three PRs and hold this one until it has been on hardware, that split is deliberate — nothing in #87–#89 depends on this.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AqfipPBp3eErDgZboooqxP