Harden Central::connect against Android 133 zombies and hangs - #2
Merged
Conversation
- `CentralConfig::connect_timeout: Option<Duration>` (default 15s) bounds
`connect()` on every backend; elapsed timeout returns a new
`BlewError::ConnectTimedOut(DeviceId)` and emits
`DeviceDisconnected { cause: Timeout }`. Replaces Linux's hardcoded 30s.
- Overlapping `connect()` calls for the same device now reject the
second caller with `BlewError::ConnectInFlight(DeviceId)` on Apple
and Android — previously the second caller silently orphaned the
first caller's completion channel. Built on a new atomic
`KeyedRequestMap::try_insert`.
- Android `disconnect()` now awaits the `onConnectionStateChange`
callback with a 2s fallback that force-closes the GATT handle via a
new Kotlin `forceClose()` entry point. Previously it returned
`Ok(())` immediately, leaking the client-IF slot when the callback
never fired.
- Kotlin: refresh()-before-close() on status=133 disconnect, plus
refresh+300ms back-off before the next `connectGatt()` in the
stale-cleanup path. Matches the canonical Android BLE workaround
for the zombie state.
- Android `central.rs` refactored from `RequestMap<u64>` +
address-keyed HashMaps to `KeyedRequestMap<String, _>` directly,
matching Apple. Eliminates the latent orphan bugs in the other
pending-op maps too.
Assisted-by: Claude:claude-opus-4-7
- jni_hooks.rs: module-level `# Safety` doc + allows for
`cast_possible_truncation` / `cast_possible_wrap` / `cast_sign_loss` /
`missing_safety_doc` — every JNI entry point shares the same "called by
the JVM through the Kotlin bindings" contract, and narrowing jint↔u16
casts on the JNI boundary are intentional.
- Rename `chr_uuid` locals to `char_id` to fix `similar_names` against
the `char_uuid: JString` JNI parameter.
- Convert manual `-> impl Future { async { … } }` definitions to
`async fn` across Android's CentralBackend / PeripheralBackend impls
(matches Linux/Apple).
- Take `&jni::errors::Error` in `jni_err` (the helpers never move out).
- Use `try_from` + explicit `BlewError::Internal` on array-size casts
into JNI (`new_object_array`, scan/advertise filters), and
`u16::try_from(...).unwrap_or(0)` for the JSON property bits.
- Merge identical `DisconnectCause::LocalClose` arms (0 | 22) and
`Ok(())` no-subscriber arms (0 | 2) per `match_same_arms`.
- Remove a `drop(JObject)` and a `mem::forget(JObject)` that clippy
flagged as no-ops — `JObject` has no `Drop` impl, so both are dead.
- Factor the L2CAP `AcceptSender` alias to clear `type_complexity`.
No behavioral change; all tests still pass (67/67) and
`cargo ndk -t arm64-v8a clippy -p blew -- -D warnings` is now clean.
Assisted-by: Claude:claude-opus-4-7
mcginty
added a commit
that referenced
this pull request
Apr 22, 2026
- `CentralConfig::connect_timeout: Option<Duration>` (default 15s) bounds
`connect()` on every backend; elapsed timeout returns a new
`BlewError::ConnectTimedOut(DeviceId)` and emits
`DeviceDisconnected { cause: Timeout }`. Replaces Linux's hardcoded 30s.
- Overlapping `connect()` calls for the same device now reject the
second caller with `BlewError::ConnectInFlight(DeviceId)` on Apple
and Android — previously the second caller silently orphaned the
first caller's completion channel. Built on a new atomic
`KeyedRequestMap::try_insert`.
- Android `disconnect()` now awaits the `onConnectionStateChange`
callback with a 2s fallback that force-closes the GATT handle via a
new Kotlin `forceClose()` entry point. Previously it returned
`Ok(())` immediately, leaking the client-IF slot when the callback
never fired.
- Kotlin: refresh()-before-close() on status=133 disconnect, plus
refresh+300ms back-off before the next `connectGatt()` in the
stale-cleanup path. Matches the canonical Android BLE workaround
for the zombie state.
- Android `central.rs` refactored from `RequestMap<u64>` +
address-keyed HashMaps to `KeyedRequestMap<String, _>` directly,
matching Apple. Eliminates the latent orphan bugs in the other
pending-op maps too.
Assisted-by: Claude:claude-opus-4-7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related connect-reliability fixes, landing together because they share surface area (
CentralConfig,BlewError, Android connect/disconnect paths).CentralConfig::connect_timeout: Option<Duration>, default 15s, threaded through all three backends. Elapsed timeout returns a new typed errorBlewError::ConnectTimedOut(DeviceId)and emitsCentralEvent::DeviceDisconnected { cause: Timeout }. Replaces Linux's prior hardcoded 30s. Opt out withSome(None)if you need the pre-0.3 unbounded behavior.onConnectionStateChange(DISCONNECTED, status=133)now calls the hiddenBluetoothGatt.refresh()beforeclose()to flush the client-side service cache.connect()'s stale-cleanup path does the same plus a 300ms back-off before issuing the freshconnectGatt()— matches the canonical Android BLE workaround and prevents the stack from silently dropping back-to-backconnectGattattempts.Central::disconnect()on Android now awaits theSTATE_DISCONNECTEDcallback with a 2s fallback that force-closes the GATT handle (new KotlinforceClose()entry point), so the client-IF slot is always freed even when the callback never arrives.connect()calls for the same device used to silently orphan the first caller's completion channel. Now rejected withBlewError::ConnectInFlight(DeviceId)on Apple and Android. Built on a new atomicKeyedRequestMap::try_insert.While fixing #3 the Android backend was refactored from
RequestMap<u64>+ address-keyed HashMaps toKeyedRequestMap<String, _>directly — same pattern Apple already uses. This also fixes latent orphan bugs inpending_ops/pending_discoverwhere a status-failure path could leave entries behind.Why
Central::connect()on Android could hang indefinitely ifonConnectionStateChangenever fired (radio busy, adapter thrash, status-133 zombie). Downstream consumers hitting this had no recovery path. The three changes here give callers bounded-time completion, a clean reconnect path after 133, and unambiguous semantics under racing callers.Field note: 15s default covers the realistic worst case on Android (SMP re-bond during ACL connect + up to 5s MTU negotiation). Tunable via
CentralConfig.connect_timeout;Nonedisables.Breaking changes
CentralConfiggained a field. Struct-literal construction needs..CentralConfig::default().BlewError::Timeoutnarrowed on Linux's connect path toBlewError::ConnectTimedOut(DeviceId). GenericTimeoutstill used bywait_ready/wait_powered. Upgrade-guide snippet inCHANGELOG.md.Test plan
mise run fmtcleanmise run lint(cargo clippy --workspace -- -D warnings) cleanmise run denycleanmise run buildcleancargo nextest run --workspace— 67/67 pass; new tests:KeyedRequestMap::try_insertsemantics,CentralConfigdefault assertsSome(15s), opt-out shape.cargo check -p blew --target aarch64-linux-androidcleanktlint crates/blew/android/**/*.ktcleanConnectTimedOutafter ~15s + immediate retry succeeds. Requires a physical device + a peer that can trigger 133.connect()on the same device → second returnsConnectInFlight, first still succeeds.adb shell dumpsys bluetooth_manager | grep clientIfafter repeated failed connects shows slots reclaimed.Reviewer notes
ConnectInFlightthere. Noted in CHANGELOG.connects.insert(...)silently evicted and orphaned — the newtry_insertpath also fixes that cross-platform.try_insert, and the refactor eliminates the orphan-on-failure footgun in the other maps for free.