The client nobody closed, closed
A Bluetooth session that lost its companion could not get it back, and the reason was one call that four separate layers each declined to make (JP, 2026-09-04). When the peripheral is what goes away, bleak fires `BLEConnection`'s dropped-link callback, which restores the connection's fields to what the caller originally passed — setting `client` back to None and letting go of the live object. `BLEConnection.disconnect` then guards on `self.client and self.client.is_connected` and has nothing to close; `ConnectionManager .disconnect` guards on a flag its own drop handler already cleared; `_force_close_transport` reaches past both, but only as far as that same `BLEConnection`, whose disconnect is the no-op above. Every guard is locally right and the sum of them is that `BleakClient.disconnect()` is never called on a link that dropped by itself. On Windows the WinRT `BluetoothLEDevice` and its GATT session then stay open for the life of the process, and every later connect to that address is served a broken service table: the link comes up and the UART subscribe fails with "Characteristic 6E400003-… was not found!". The device advertises at -61 dBm, answers a hand-written APP_START over raw GATT with a valid SELF_INFO, and is unreachable to this app until it restarts — while a fresh process connects to it in 0.6s. That asymmetry is the whole tell, and it is what turned every reconnect attempt into a fast, inexplicable failure. So the client is ours now: taken at connect while the library still has one to give, and closed unconditionally at teardown — never guarded on `is_connected`, because the case that matters is exactly the one where it is already False. Bounded and silent, since a teardown may not raise or hang, and a no-op on the transports that have no client to hold. Measured on the real companion: the session drops, is heard again 7.2s after the device comes back, and is rebuilt 0.97s later — 8.3s from drop to live, where it previously never recovered at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
c090db829c
commit
0b719a4736
2 files changed
+171
-5
No files matched your search
@@ -1021,6 +1021,10 @@ class MeshCoreDevice(Device):
|
||||
self._host = host
|
||||
self._tcp_port = tcp_port
|
||||
self._mc = None # type: ignore[var-annotated] # meshcore.MeshCore
|
||||
#: The ``bleak.BleakClient`` behind a BLE link, kept because ``meshcore`` lets go of
|
||||
#: its own reference the moment the peripheral vanishes — see :meth:`_release_ble_client`
|
||||
#: for why nothing else can close it, and what that costs when it stays open.
|
||||
self._ble_client = None # type: ignore[var-annotated] # bleak.BleakClient
|
||||
# Serializes channel reads. The meshcore library's get_channel waits for "the next
|
||||
# CHANNEL_INFO event" with no correlation to the index it asked for, and the dispatcher
|
||||
# fans that event to *every* in-flight waiter — so two concurrent reads both resolve on
|
||||
@@ -1245,10 +1249,17 @@ class MeshCoreDevice(Device):
|
||||
try:
|
||||
started = await mc.connect()
|
||||
except BaseException:
|
||||
self._ble_client = getattr(connection, "client", None)
|
||||
await MeshCoreDevice._discard_meshcore(mc)
|
||||
await self._release_ble_client()
|
||||
raise
|
||||
# Take our own reference to the bleak client now, while ``BLEConnection`` still holds
|
||||
# one. It drops it the instant the peripheral goes away, and by then nothing else can
|
||||
# reach the object that needs closing (see :meth:`_release_ble_client`).
|
||||
self._ble_client = getattr(connection, "client", None)
|
||||
if started is None:
|
||||
await MeshCoreDevice._discard_meshcore(mc)
|
||||
await self._release_ble_client()
|
||||
return None
|
||||
return mc
|
||||
|
||||
@@ -1619,24 +1630,73 @@ class MeshCoreDevice(Device):
|
||||
process. When the graceful path doesn't return in time it is cancelled and the
|
||||
teardown is forced instead: the dispatcher task is cancelled synchronously and the
|
||||
raw transport is closed directly (also bounded), so the port/link is still released.
|
||||
|
||||
Over Bluetooth the ``bleak`` client is then closed directly as well, because none of
|
||||
the above reaches it once the peripheral is the thing that went away — see
|
||||
:meth:`_release_ble_client`.
|
||||
"""
|
||||
if self._mc is None:
|
||||
await self._release_ble_client() # a link that dropped before we ever tore it down
|
||||
return
|
||||
mc, self._mc = self._mc, None
|
||||
disconnect = getattr(mc, "disconnect", None)
|
||||
if disconnect is None:
|
||||
await self._release_ble_client()
|
||||
return
|
||||
try:
|
||||
await asyncio.wait_for(disconnect(), timeout=_DISCONNECT_TIMEOUT_S)
|
||||
return
|
||||
except asyncio.TimeoutError:
|
||||
_log.debug("graceful disconnect timed out; forcing transport teardown")
|
||||
# Forced teardown: cancel the (possibly wedged) dispatcher task without awaiting
|
||||
# the deadlocked join, then close the underlying transport so the serial port /
|
||||
# BLE link is actually released. Best-effort — nothing here may block the exit.
|
||||
await MeshCoreDevice._force_close_transport(mc)
|
||||
except Exception as exc: # noqa: BLE001 - teardown must not raise; fall to forced path
|
||||
_log.debug("graceful disconnect failed (%s); forcing transport teardown", exc)
|
||||
# Forced teardown: cancel the (possibly wedged) dispatcher task without awaiting the
|
||||
# deadlocked join, then close the underlying transport so the serial port / BLE link
|
||||
# is actually released. Every step is best-effort — nothing here may block the exit.
|
||||
await MeshCoreDevice._force_close_transport(mc)
|
||||
await MeshCoreDevice._force_close_transport(mc)
|
||||
await self._release_ble_client()
|
||||
|
||||
async def _release_ble_client(self) -> None:
|
||||
"""Close the ``bleak`` client itself, whatever the library believes about its state.
|
||||
|
||||
The one teardown step that cannot be delegated, and the reason a Bluetooth session
|
||||
could not be rebuilt after the companion was switched off. Every layer above declines
|
||||
to act once the *peripheral* is what went away, each for its own locally-sensible
|
||||
reason:
|
||||
|
||||
* ``BLEConnection.handle_disconnect`` — bleak's own dropped-link callback — restores
|
||||
the connection's fields to what the caller originally passed, which sets
|
||||
``self.client`` back to ``None``. The live client object is simply let go of.
|
||||
* ``BLEConnection.disconnect`` then guards on ``self.client and
|
||||
self.client.is_connected``, so it has nothing to close and does nothing.
|
||||
* ``ConnectionManager.disconnect`` guards on ``self._is_connected``, which its own
|
||||
drop handler already cleared, so it does nothing either.
|
||||
* :meth:`_force_close_transport` reaches past both — but only as far as that same
|
||||
``BLEConnection``, whose ``disconnect`` is the no-op above.
|
||||
|
||||
So on a drop nobody ever calls ``BleakClient.disconnect()``, and on Windows the WinRT
|
||||
``BluetoothLEDevice`` and its GATT session stay open for the life of the process. The
|
||||
next connect to that address is then served a broken service table: the link comes up,
|
||||
and subscribing to the UART characteristic fails with *"Characteristic
|
||||
6E400003-… was not found!"* — a device that is advertising, healthy, and unreachable
|
||||
for as long as the app keeps running. A fresh process connects to it perfectly.
|
||||
|
||||
Hence our own reference, taken at connect (see :meth:`_connect_owned_ble`) while the
|
||||
library still has one to give, and closed here **unconditionally** — never guarded on
|
||||
``is_connected``, since the case that matters is precisely the one where it is already
|
||||
``False``. Bounded and silent: the link is already gone, and a teardown may not raise
|
||||
or hang. Idempotent, and a no-op on serial and TCP, which have no client to hold.
|
||||
"""
|
||||
client, self._ble_client = self._ble_client, None
|
||||
if client is None:
|
||||
return
|
||||
disconnect = getattr(client, "disconnect", None)
|
||||
if disconnect is None: # pragma: no cover - every bleak client has one
|
||||
return
|
||||
try:
|
||||
await asyncio.wait_for(disconnect(), timeout=_FORCE_DISCONNECT_TIMEOUT_S)
|
||||
except Exception as exc: # noqa: BLE001 - the link is already gone; best-effort
|
||||
_log.debug("releasing the bleak client failed: %s", exc)
|
||||
|
||||
def _require(self): # type: ignore[no-untyped-def]
|
||||
"""Return the live client or raise if not connected."""
|
||||
|
||||
@@ -1108,3 +1108,109 @@ async def test_release_link_is_idempotent_and_holds_the_resume_intent(tmp_path:
|
||||
assert ctx.events.active and ctx.monitor.active # the held intent survived both
|
||||
finally:
|
||||
await ctx.aclose()
|
||||
|
||||
|
||||
|
||||
class _AbandonedBleakClient:
|
||||
"""A ``bleak`` client whose peripheral vanished: already down, and never handed back.
|
||||
|
||||
Mirrors what the meshcore transport leaves behind on a dropped link — ``is_connected`` is
|
||||
already ``False``, so every guarded teardown in the library declines to touch it.
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.is_connected = False
|
||||
self.disconnect_calls = 0
|
||||
|
||||
async def disconnect(self) -> None:
|
||||
self.disconnect_calls += 1
|
||||
|
||||
|
||||
class _DroppedBleConnection:
|
||||
"""meshcore's ``BLEConnection`` after its dropped-link callback ran: client reference gone."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.client = None # handle_disconnect restored this to the constructor's value
|
||||
self.disconnect_calls = 0
|
||||
|
||||
async def disconnect(self) -> None:
|
||||
self.disconnect_calls += 1 # guards on self.client — reaches nothing
|
||||
|
||||
|
||||
class _DroppedMeshCore:
|
||||
"""A meshcore client whose connection manager also believes it is already disconnected."""
|
||||
|
||||
def __init__(self, connection: _DroppedBleConnection) -> None:
|
||||
self.connection_manager = SimpleNamespace(connection=connection)
|
||||
self.disconnected = False
|
||||
|
||||
async def disconnect(self) -> None:
|
||||
self.disconnected = True # guards on _is_connected — reaches nothing
|
||||
|
||||
def stop(self) -> None:
|
||||
pass
|
||||
|
||||
|
||||
async def test_a_vanished_peripheral_still_gets_its_bleak_client_closed() -> None:
|
||||
"""The client is released even though every library guard says there is nothing to close."""
|
||||
dev = connection.MeshCoreDevice(transport="ble", address="AA:BB:00:00:00:01")
|
||||
dropped = _DroppedBleConnection()
|
||||
dev._mc = _DroppedMeshCore(dropped)
|
||||
abandoned = _AbandonedBleakClient()
|
||||
dev._ble_client = abandoned # what we kept hold of at connect
|
||||
|
||||
await dev.disconnect()
|
||||
|
||||
# The library's own teardown reached nothing — the connection had already let the client go.
|
||||
assert dropped.client is None
|
||||
# Ours closed it anyway, which is what frees the WinRT handles the next connect needs.
|
||||
assert abandoned.disconnect_calls == 1
|
||||
assert dev._ble_client is None # and it is not closed twice
|
||||
assert dev._mc is None
|
||||
|
||||
|
||||
async def test_releasing_the_bleak_client_is_idempotent_and_never_raises() -> None:
|
||||
"""A second disconnect finds nothing to do, and a failing close is swallowed."""
|
||||
dev = connection.MeshCoreDevice(transport="ble", address="AA:BB:00:00:00:01")
|
||||
|
||||
await dev.disconnect() # nothing connected at all
|
||||
assert dev._ble_client is None
|
||||
|
||||
class _RefusesToClose:
|
||||
is_connected = False
|
||||
|
||||
async def disconnect(self) -> None:
|
||||
raise OSError("the handle is invalid")
|
||||
|
||||
dev._ble_client = _RefusesToClose()
|
||||
await dev.disconnect() # must not raise: teardown is best-effort
|
||||
assert dev._ble_client is None
|
||||
|
||||
|
||||
async def test_a_healthy_bluetooth_teardown_still_releases_the_client() -> None:
|
||||
"""The graceful path runs as before, and the client is released after it."""
|
||||
dev = connection.MeshCoreDevice(transport="ble", address="AA:BB:00:00:00:01")
|
||||
|
||||
class _HealthyMeshCore:
|
||||
def __init__(self) -> None:
|
||||
self.disconnected = False
|
||||
self.force_stopped = False
|
||||
self.connection_manager = SimpleNamespace(connection=self)
|
||||
|
||||
async def disconnect(self) -> None:
|
||||
self.disconnected = True
|
||||
|
||||
def stop(self) -> None:
|
||||
self.force_stopped = True
|
||||
|
||||
healthy = _HealthyMeshCore()
|
||||
dev._mc = healthy
|
||||
live = _AbandonedBleakClient()
|
||||
live.is_connected = True
|
||||
dev._ble_client = live
|
||||
|
||||
await dev.disconnect()
|
||||
|
||||
assert healthy.disconnected
|
||||
assert not healthy.force_stopped # the graceful path was enough, as it always was
|
||||
assert live.disconnect_calls == 1 # and the client is released regardless
|
||||
Reference in new issue
Block a user