fix: prevent re-entrant disconnect callbacks #4

Merged

schwifty merged 2 commits from schwifty/fix-disconnect-race into main

2026-03-21 23:37:08 +00:00

schwifty commented

2026-03-21 23:01:46 +00:00

Collaborator

Problem

Still getting "Unexpected disconnect" after PR #3 landed the SaveConfig fix.

Root Causes Found

Race condition in succeed()/fail(): When succeed() is called from didDisconnectPeripheral, it calls cancelPeripheralConnection on an already-disconnected peripheral. On some iOS versions, this can trigger a SECOND didDisconnectPeripheral callback. If cleanup() has already reset the queue/index state, the second callback could hit the wrong path.
Disconnect during device info read: After auth succeeds, there's a 3-second window reading the beacon's MAC address. During this, state is still .authenticating, so any disconnect falls through to the generic "Unexpected disconnect" handler.

Fix

isTerminating flag: Guards succeed() and fail() against double invocation from racing CoreBluetooth callbacks. Checked at the top of didDisconnectPeripheral to skip expected cleanup disconnects.
Peripheral state check: Only calls cancelPeripheralConnection when peripheral.state == .connected (avoids triggering spurious callbacks on already-disconnected peripherals).
Device info read handling: Specific error message when disconnect happens during post-auth device info read.
State in error messages: Unexpected disconnect now includes the provisioner state for debugging.

Testing

Needs Xcode rebuild + test with physical beacon.

## Problem Still getting "Unexpected disconnect" after PR #3 landed the SaveConfig fix. ## Root Causes Found 1. **Race condition in succeed()/fail()**: When `succeed()` is called from `didDisconnectPeripheral`, it calls `cancelPeripheralConnection` on an already-disconnected peripheral. On some iOS versions, this can trigger a SECOND `didDisconnectPeripheral` callback. If `cleanup()` has already reset the queue/index state, the second callback could hit the wrong path. 2. **Disconnect during device info read**: After auth succeeds, there's a 3-second window reading the beacon's MAC address. During this, `state` is still `.authenticating`, so any disconnect falls through to the generic "Unexpected disconnect" handler. ## Fix - **`isTerminating` flag**: Guards `succeed()` and `fail()` against double invocation from racing CoreBluetooth callbacks. Checked at the top of `didDisconnectPeripheral` to skip expected cleanup disconnects. - **Peripheral state check**: Only calls `cancelPeripheralConnection` when `peripheral.state == .connected` (avoids triggering spurious callbacks on already-disconnected peripherals). - **Device info read handling**: Specific error message when disconnect happens during post-auth device info read. - **State in error messages**: Unexpected disconnect now includes the provisioner state for debugging. ## Testing Needs Xcode rebuild + test with physical beacon.

schwifty added 1 commit 2026-03-21 23:01:47 +00:00

fix: prevent re-entrant disconnect callbacks and handle all disconnect states 54fa973d34

- Add isTerminating flag to guard succeed()/fail() against double invocation
  from racing didWriteValueFor + didDisconnectPeripheral callbacks
- Only call cancelPeripheralConnection when peripheral.state == .connected
  (avoids triggering spurious didDisconnectPeripheral on already-disconnected peripheral)
- Handle disconnect during device info read (post-auth) with specific error message
- Include state info in unexpected disconnect errors for easier debugging
- Early-return structure in disconnect handler for clearer control flow

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

schwifty added 1 commit 2026-03-21 23:03:47 +00:00

docs: fix misleading comment on post-auth disconnect path 58be00cb38

The comment said "treat as non-fatal" but the code calls fail() — which
is correct behavior since we can't write config without a connection.
Updated comment to accurately describe the fail-with-retry-prompt flow.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>