From dbe8b154797b84afd9533e78279d20aab48efb4f Mon Sep 17 00:00:00 2001 From: Johnny D Date: Wed, 9 Sep 2026 14:41:19 -0400 Subject: [PATCH] fix(session): retire callbacks before teardown and require usable input for readiness Guard late notifications, queued commands and timers after session retirement. Clear notify completion before invocation to preserve reentrant replacement. Require isNotifying on essential channels and one existing-decoder input report before readiness, retaining the established keep-alive and handshake bytes. Eleven synthetic production-session tests pass locally; five new targeted cases fail against the prior session source. Full Apple-framework build and regressions remain the hosted gate. No new connection deadline or protocol. --- .../Bluetooth/ControllerSession.swift | 79 ++++++++++++++----- docs/session-retirement.md | 30 +++++++ tests/session/SessionTests.swift | 72 +++++++++++++++++ 3 files changed, 160 insertions(+), 21 deletions(-) create mode 100644 docs/session-retirement.md diff --git a/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift b/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift index 598f7d5..888190f 100644 --- a/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift +++ b/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift @@ -68,6 +68,9 @@ final class ControllerSession: NSObject, @unchecked Sendable { private var chars: [UUID: CBCharacteristic] = [:] private var handshakeStarted = false + private var ended = false + private var handshakeComplete = false + private var readyReported = false private var pendingCommand: (id: UInt8, completion: (Data?) -> Void)? private var commandTimeout: DispatchWorkItem? private var handshakeSteps: [(String, (@escaping (Bool) -> Void) -> Void)] = [] @@ -125,11 +128,18 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// Called by the engine once CoreBluetooth reports the connect. func begin() { + guard !ended else { return } log(.info, "slot \(slot + 1): discovering services") peripheral.discoverServices(nil) } func teardown() { + guard !ended else { return } + ended = true + notifyCompletion = nil + handshakeSteps.removeAll() + onState = nil + onRSSI = nil keepAliveTimer?.cancel() keepAliveTimer = nil commandTimeout?.cancel() @@ -143,12 +153,14 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func fail(_ reason: String) { + guard !ended else { return } log(.error, "slot \(slot + 1): \(reason)") teardown() delegate?.sessionFailed(self, reason: reason) } private func runHandshake() { + guard !ended else { return } // Order matters and mirrors the console: command-response subscribe // must precede any command; identity before vibration char choice. handshakeSteps = [ @@ -163,15 +175,17 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func advanceHandshake() { + guard !ended else { return } guard !handshakeSteps.isEmpty else { + handshakeComplete = true + // Keep the existing keep-alive while awaiting the first report. startKeepAlive() - log(.info, "slot \(slot + 1): handshake complete — \(displayName) serial \(serialNumber)") - delegate?.sessionReady(self) + if announceReady() { onState?(slot, state) } return } let (name, step) = handshakeSteps.removeFirst() step { [weak self] ok in - guard let self else { return } + guard let self, !self.ended else { return } if ok { self.advanceHandshake() } else { @@ -180,6 +194,16 @@ final class ControllerSession: NSObject, @unchecked Sendable { } } + /// Notification subscription alone is not usable controller input. + @discardableResult + private func announceReady() -> Bool { + guard !ended, handshakeComplete, reportCount > 0, !readyReported else { return false } + readyReported = true + log(.info, "slot \(slot + 1): handshake and first input complete — \(displayName)") + delegate?.sessionReady(self) + return true + } + // MARK: Handshake steps private func stepReadInfo(_ done: @escaping (Bool) -> Void) { @@ -196,7 +220,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private func stepReadCalibration(_ done: @escaping (Bool) -> Void) { readStickCalibration(user: Switch2.Address.userStick1, factory: Switch2.Address.factoryStick1) { [weak self] cal in - guard let self else { return } + guard let self, !self.ended else { return } self.leftCal = cal guard self.model.hasSecondStick else { done(true) // single-stick unit: slot-2 holds no valid data @@ -204,7 +228,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } self.readStickCalibration(user: Switch2.Address.userStick2, factory: Switch2.Address.factoryStick2) { [weak self] cal in - guard let self else { return } + guard let self, !self.ended else { return } self.rightCal = cal done(true) // calibration is best-effort; defaults are usable } @@ -214,7 +238,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private func readStickCalibration(user: UInt32, factory: UInt32, _ done: @escaping (Switch2.StickCalibration?) -> Void) { readMemory(length: 0x0B, address: user) { [weak self] data in - guard let self else { return } + guard let self, !self.ended else { return } if let data, !Switch2.StickCalibration.isBlank(data) { done(Switch2.StickCalibration(data: data)) return @@ -274,7 +298,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private func writeCommand(_ command: UInt8, _ subcommand: UInt8, _ data: Data, flag: UInt8 = 0x01, completion: @escaping (Data?) -> Void) { - guard let writeChar = chars[Switch2.GATT.commandWrite] else { + guard !ended, let writeChar = chars[Switch2.GATT.commandWrite] else { completion(nil); return } guard pendingCommand == nil else { @@ -299,7 +323,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private func handleCommandResponse(_ data: Data) { // An unrelated notification must not consume the command or its timer. - guard let pending = pendingCommand, data.count >= 8, + guard !ended, let pending = pendingCommand, data.count >= 8, data[data.startIndex] == pending.id, data[data.startIndex + 1] == 0x01 || data[data.startIndex + 1] == 0x02 else { return } commandTimeout?.cancel() @@ -349,7 +373,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// bypasses persisted patterns. Restores normal LEDs when `nil`. func setRawLEDs(_ pattern: UInt8?) { queue.async { [weak self] in - guard let self else { return } + guard let self, !self.ended else { return } if let pattern { self.writeCommand(Switch2.Command.leds, Switch2.Subcommand.ledsSetPlayer, Data([pattern, 0, 0, 0])) { _ in } @@ -367,7 +391,10 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// Read the current RSSI; result arrives via the rssi callback. var onRSSI: ((Int) -> Void)? func requestRSSI() { - queue.async { [weak self] in self?.peripheral.readRSSI() } + queue.async { [weak self] in + guard let self, !self.ended else { return } + self.peripheral.readRSSI() + } } // MARK: - Experiments (NFC probing, audio capture) @@ -392,7 +419,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private(set) var promiscuousNotify = false func setPromiscuousNotify(_ enabled: Bool) { queue.async { [weak self] in - guard let self else { return } + guard let self, !self.ended else { return } self.promiscuousNotify = enabled let known: Set = [Switch2.GATT.inputReport, Switch2.GATT.commandResponse, @@ -414,7 +441,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// queue only). Returns false when the characteristic is absent. @discardableResult func writeAudioFrame(_ data: Data) -> Bool { - guard let ch = chars[Self.audioOutputUUID] else { return false } + guard !ended, let ch = chars[Self.audioOutputUUID] else { return false } peripheral.writeValue(data, for: ch, type: .withoutResponse) lastWriteAt = CFAbsoluteTimeGetCurrent() return true @@ -463,7 +490,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private(set) var audioExperimentName: String? func beginAudioExperiment(_ name: String) -> Bool { dispatchPrecondition(condition: .onQueue(queue)) - guard audioExperimentName == nil else { return false } + guard !ended, audioExperimentName == nil else { return false } audioExperimentName = name return true } @@ -487,7 +514,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { next: @escaping () -> Data?, done: @escaping (AudioStreamStats) -> Void) { queue.async { [weak self] in - guard let self else { return } + guard let self, !self.ended else { return } self.audioStreamTimer?.cancel() self.audioStreamQueue.removeAll() self.audioStreamStats = AudioStreamStats(chunkLimit: self.audioWriteChunkLimit) @@ -533,7 +560,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// Write queued chunks until the stack refuses; `peripheralIsReady` /// re-enters. Runs on the Bluetooth queue only. fileprivate func drainAudioStream() { - guard audioStreamNext != nil, let ch = chars[Self.audioOutputUUID] else { return } + guard !ended, audioStreamNext != nil, let ch = chars[Self.audioOutputUUID] else { return } while !audioStreamQueue.isEmpty { guard peripheral.canSendWriteWithoutResponse else { audioStreamStats.stalls += 1 @@ -570,7 +597,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// Returns false via completion when the firmware doesn't expose it. func setAudioCapture(_ enabled: Bool, completion: @escaping (Bool) -> Void) { queue.async { [weak self] in - guard let self, let ch = self.chars[Self.audioInputUUID] else { + guard let self, !self.ended, let ch = self.chars[Self.audioInputUUID] else { completion(false); return } self.peripheral.setNotifyValue(enabled, for: ch) completion(true) @@ -581,13 +608,14 @@ final class ControllerSession: NSObject, @unchecked Sendable { func setRumble(strong: Double, weak: Double) { queue.async { [weak self] in - guard let self else { return } + guard let self, !self.ended else { return } self.rumbleTarget = (strong, weak) self.rumbleSetAt = CFAbsoluteTimeGetCurrent() } } private func startKeepAlive() { + guard !ended, keepAliveTimer == nil else { return } let timer = DispatchSource.makeTimerSource(queue: queue) timer.schedule(deadline: .now() + 0.05, repeating: 0.05) timer.setEventHandler { [weak self] in self?.maintainTick() } @@ -596,6 +624,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func maintainTick() { + guard !ended else { return } let now = CFAbsoluteTimeGetCurrent() var (strong, weakMag) = rumbleTarget // Failsafe: rumble intents expire after 0.5 s so a crashed consumer @@ -617,7 +646,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func writeMotor(_ vib: Switch2.Vibration) { - guard model.hasHDRumble, + guard !ended, model.hasHDRumble, let motor = chars[Switch2.GATT.vibration(for: model)] else { return } let packet = Switch2.motorPacket(vib, packetID: vibrationPacketID, model: model) vibrationPacketID &+= 1 @@ -638,7 +667,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func handleInputReport(_ data: Data) { - guard let report = Switch2.InputReport(data: data) else { return } + guard !ended, let report = Switch2.InputReport(data: data) else { return } let now = CFAbsoluteTimeGetCurrent() if lastReportAt > 0, now - lastReportAt > 0.100 { gapCount += 1 @@ -694,6 +723,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } state = s + announceReady() onState?(slot, s) // Battery / rate refresh for the UI at ~1 Hz. @@ -712,6 +742,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { extension ControllerSession: CBPeripheralDelegate { func peripheral(_ peripheral: CBPeripheral, didDiscoverServices error: Error?) { + guard !ended else { return } if let error { fail("service discovery: \(error.localizedDescription)"); return } for service in peripheral.services ?? [] { peripheral.discoverCharacteristics(nil, for: service) @@ -721,6 +752,7 @@ extension ControllerSession: CBPeripheralDelegate { func peripheral(_ peripheral: CBPeripheral, didDiscoverCharacteristicsFor service: CBService, error: Error?) { + guard !ended else { return } if let error { fail("characteristic discovery: \(error.localizedDescription)"); return } for ch in service.characteristics ?? [] { if let uuid = UUID(uuidString: ch.uuid.uuidString) { @@ -741,6 +773,7 @@ extension ControllerSession: CBPeripheralDelegate { func peripheral(_ peripheral: CBPeripheral, didUpdateNotificationStateFor characteristic: CBCharacteristic, error: Error?) { + guard !ended else { return } let uuid = UUID(uuidString: characteristic.uuid.uuidString) if let error { // Only the essential channels are fatal — an experimental @@ -752,11 +785,15 @@ extension ControllerSession: CBPeripheralDelegate { } return } + if uuid == Switch2.GATT.commandResponse || uuid == Switch2.GATT.inputReport { + guard characteristic.isNotifying else { fail("essential notifications stopped"); return } + } if uuid == Switch2.GATT.commandResponse { runHandshake() } else if uuid == Switch2.GATT.inputReport { - notifyCompletion?(true) + let completion = notifyCompletion notifyCompletion = nil + completion?(true) } } @@ -772,7 +809,7 @@ extension ControllerSession: CBPeripheralDelegate { func peripheral(_ peripheral: CBPeripheral, didUpdateValueFor characteristic: CBCharacteristic, error: Error?) { - guard error == nil, let data = characteristic.value else { return } + guard !ended, error == nil, let data = characteristic.value else { return } let uuid = UUID(uuidString: characteristic.uuid.uuidString) if uuid == Switch2.GATT.inputReport { handleInputReport(data) diff --git a/docs/session-retirement.md b/docs/session-retirement.md new file mode 100644 index 0000000..f996f95 --- /dev/null +++ b/docs/session-retirement.md @@ -0,0 +1,30 @@ +# Session retirement and usable readiness + +A controller session now has an explicit terminal state. Teardown marks it +ended before clearing pending notifications, callbacks, handshake steps and +timers. Late CoreBluetooth notifications, queued commands/rumble and audio +work cannot restart or emit input from that retired session. Essential +notification-state callbacks must actually report isNotifying. Completion +storage is cleared before invocation so a reentrant callback cannot erase new +work. + +Readiness now requires both the existing handshake and a report accepted by +the existing decoder. No report lengths, controller bytes, pairing sequence, +GATT identifiers or connection timeouts are changed. The normal keep-alive +starts when the handshake completes even while awaiting input. If input +arrived during the handshake, its latest state is delivered when readiness is +announced. Readiness is announced once, before delivering the first usable +state, allowing the engine to attach output without losing that state. + +Eleven production-session tests pass with fake CoreBluetooth boundaries. Five +new selected failure cases were first run against the prior PR #3 source and +failed: retired notifications, retired input/commands, false readiness, +disabled notifications, and reentrant completion. Additional tests check early +input and stopped keep-alives. The complete app is compiled separately with +Apple SDKs; these fixtures are synthetic, not controller captures. + +Run bash tests/session/run.sh. Physical pairing/reconnect and keep-alive +behavior still need acceptance on the target firmware. Engine-level slot +replacement and delayed player-number callbacks remain separate ownership +boundaries; this session-local guard is not a claim that every lifecycle race +in the application has been eliminated. diff --git a/tests/session/SessionTests.swift b/tests/session/SessionTests.swift index d94aa53..a6b1805 100644 --- a/tests/session/SessionTests.swift +++ b/tests/session/SessionTests.swift @@ -8,6 +8,13 @@ final class Delegate: ControllerSessionDelegate { func sessionDidUpdateState(_ session: ControllerSession) {} } +final class StateCounter: @unchecked Sendable { + private let lock = NSLock() + private var count = 0 + func increment() { lock.lock(); defer { lock.unlock() }; count += 1 } + var value: Int { lock.lock(); defer { lock.unlock() }; return count } +} + @main enum SessionTests { static func fixture() -> (ControllerSession, CBPeripheral, DispatchQueue, Delegate) { @@ -24,6 +31,71 @@ enum SessionTests { func run(_ name: String, _ test: () -> Void) { if selected == "all" || selected == name { test(); print("PASS \(name)") } } + run("retired-notification") { + let (s, _, _, d) = fixture(); defer { _ = d } + var calls = 0 + s.notifyCompletion = { _ in calls += 1 } + s.teardown() + let input = s.chars[Switch2.GATT.inputReport]! + input.isNotifying = true + s.peripheral(s.peripheral, didUpdateNotificationStateFor: input, error: nil) + precondition(calls == 0, "A retired subscription callback resumed its handshake") + } + run("retired-input-command") { + let (s, p, q, d) = fixture(); defer { _ = d } + let states = StateCounter() + s.onState = { _, _ in states.increment() } + s.teardown() + s.handleInputReport(Data(repeating: 0, count: 63)) + s.experimentalCommand(9, 7, payload: Data()) { _ in } + q.sync {} + precondition(states.value == 0 && p.writes.isEmpty, "Retired input/commands must not reach outputs") + } + run("ready-needs-input") { + let (s, _, _, d) = fixture(); defer { s.teardown() } + s.advanceHandshake() + precondition(d.ready == 0, "Subscription/handshake without a valid input report is not ready") + s.handleInputReport(Data(repeating: 0, count: 10)) + precondition(d.ready == 0) + s.handleInputReport(Data(repeating: 0, count: 63)) + precondition(d.ready == 1) + s.handleInputReport(Data(repeating: 0, count: 63)) + precondition(d.ready == 1) + } + run("early-report-is-delivered") { + let (s, _, _, d) = fixture(); defer { s.teardown() } + s.handleInputReport(Data(repeating: 0, count: 63)) + precondition(d.ready == 0) + let states = StateCounter() + s.onState = { _, _ in states.increment() } + s.advanceHandshake() + precondition(d.ready == 1 && states.value == 1, "Input received during handshake must not be lost") + } + run("retired-keepalive") { + let (s, p, q, d) = fixture(); defer { _ = d } + q.sync { + s.advanceHandshake() + s.teardown() + s.maintainTick() + s.begin() + precondition(s.keepAliveTimer == nil && p.writes.isEmpty) + } + } + run("notification-disabled") { + let (s, _, _, d) = fixture(); defer { s.teardown() } + let input = s.chars[Switch2.GATT.inputReport]! + input.isNotifying = false + s.peripheral(s.peripheral, didUpdateNotificationStateFor: input, error: nil) + precondition(d.failures == 1, "Disabled essential notifications are not success") + } + run("notification-completion-reentrancy") { + let (s, _, _, d) = fixture(); defer { s.teardown(); _ = d } + let input = s.chars[Switch2.GATT.inputReport]! + input.isNotifying = true + s.notifyCompletion = { _ in s.notifyCompletion = { _ in } } + s.peripheral(s.peripheral, didUpdateNotificationStateFor: input, error: nil) + precondition(s.notifyCompletion != nil, "A completed callback erased replacement work") + } run("rumble") { for model in Switch2.Model.allCases { let (s, p, q, d) = fixture(); defer { s.teardown(); _ = d }