diff --git a/meshtastic/ble_interface.py b/meshtastic/ble_interface.py index 64a63616f..c4c27b9d2 100644 --- a/meshtastic/ble_interface.py +++ b/meshtastic/ble_interface.py @@ -7,6 +7,7 @@ import sys import time import io +from concurrent.futures import TimeoutError as FutureTimeoutError from threading import Thread, Event from typing import List, Optional @@ -24,6 +25,12 @@ FROMNUM_UUID = "ed9da18c-a800-4f66-a670-aa7547e34453" LEGACY_LOGRADIO_UUID = "6c6fd238-78fa-436b-aacf-15c5be1ef2e2" LOGRADIO_UUID = "5a3d6e49-06e6-4423-9944-e9de8cdf9547" + +# Upper bound (seconds) on how long we wait for bleak to tear a connection +# down. bleak's disconnect can stall indefinitely on some backends, so we cap +# it to guarantee close() returns instead of hanging forever. +BLE_DISCONNECT_TIMEOUT = 5.0 + logger = logging.getLogger(__name__) @@ -204,6 +211,7 @@ def _receiveFromRadioImpl(self) -> None: logger.debug(f"BLE client is None, shutting down") self._want_receive = False continue + b = b"" try: b = bytes(self.client.read_gatt_char(FROMRADIO_UUID)) except BleakDBusError as e: @@ -299,7 +307,15 @@ def connect(self, **kwargs): # pylint: disable=C0116 return self.async_await(self.bleak_client.connect(**kwargs)) def disconnect(self, **kwargs): # pylint: disable=C0116 - self.async_await(self.bleak_client.disconnect(**kwargs)) + # bleak's disconnect can stall indefinitely on some backends; bound it + # so BLEInterface.close() can never hang forever waiting on teardown. + try: + self.async_await( + self.bleak_client.disconnect(**kwargs), + timeout=BLE_DISCONNECT_TIMEOUT, + ) + except (FutureTimeoutError, BleakError) as e: + logger.warning(f"BLE disconnect did not complete cleanly: {e}") def read_gatt_char(self, *args, **kwargs): # pylint: disable=C0116 return self.async_await(self.bleak_client.read_gatt_char(*args, **kwargs)) @@ -316,7 +332,9 @@ def start_notify(self, *args, **kwargs): # pylint: disable=C0116 def close(self): # pylint: disable=C0116 self.async_run(self._stop_event_loop()) - self._eventThread.join() + # The event loop thread is a daemon; if it fails to stop promptly we + # must not block the caller forever, so join with a bounded timeout. + self._eventThread.join(timeout=BLE_DISCONNECT_TIMEOUT) def __enter__(self): return self @@ -336,7 +354,13 @@ def async_await(self, coro, timeout=None): # pylint: disable=C0116 # On macOS without debug logging, callbacks may not be delivered # unless we trigger some I/O. This is a known quirk of CoreBluetooth. sys.stdout.flush() - result = future.result(timeout) + try: + result = future.result(timeout) + except FutureTimeoutError: + # The coroutine is still queued/running on the event loop; cancel it + # so a stalled call (e.g. a hung disconnect) is not left pending. + future.cancel() + raise logger.debug("async_await: complete") return result diff --git a/meshtastic/tests/test_ble_interface.py b/meshtastic/tests/test_ble_interface.py index 0b725e65c..d65b26c1f 100644 --- a/meshtastic/tests/test_ble_interface.py +++ b/meshtastic/tests/test_ble_interface.py @@ -1,11 +1,12 @@ """Meshtastic unit tests for ble_interface.py""" +from concurrent.futures import TimeoutError as FutureTimeoutError from unittest.mock import MagicMock, patch import pytest -from bleak.exc import BleakError +from bleak.exc import BleakDBusError, BleakError -from ..ble_interface import BLEInterface +from ..ble_interface import BLE_DISCONNECT_TIMEOUT, BLEClient, BLEInterface @pytest.mark.unit @@ -67,3 +68,65 @@ def test_ble_receive_wraps_unexpected_bleak_error_with_kind(): with pytest.raises(BLEInterface.BLEError) as excinfo: iface._receiveFromRadioImpl() assert excinfo.value.kind == BLEInterface.BLEError.READ_ERROR + + +@pytest.mark.unit +def test_ble_receive_disconnect_mid_read_unwinds_cleanly(): + """A disconnect mid-read (BleakDBusError) must stop the receive loop + without raising UnboundLocalError on the `b` read buffer.""" + iface = object.__new__(BLEInterface) + iface.should_read = True + iface._want_receive = True + iface.client = MagicMock() + iface.client.read_gatt_char.side_effect = BleakDBusError( + "org.bluez.Error.Failed", [] + ) + # Must return normally (no UnboundLocalError) and halt the loop. + iface._receiveFromRadioImpl() + assert iface._want_receive is False + + +@pytest.mark.unit +def test_ble_client_disconnect_swallows_stalled_teardown(): + """BLEClient.disconnect must bound the wait with BLE_DISCONNECT_TIMEOUT and + not propagate a stalled-teardown timeout, so BLEInterface.close() can always + finish.""" + client = object.__new__(BLEClient) + client.bleak_client = MagicMock() + with patch.object( + BLEClient, "async_await", side_effect=FutureTimeoutError() + ) as async_await: + client.disconnect() # must not raise + # the wait must actually be bounded, not left unbounded + assert async_await.call_args.kwargs["timeout"] == BLE_DISCONNECT_TIMEOUT + with patch.object(BLEClient, "async_await", side_effect=BleakError("gone")): + client.disconnect() # must not raise + + +@pytest.mark.unit +def test_ble_client_close_bounds_event_thread_join(): + """BLEClient.close must bound the event-loop thread join so a stuck loop + cannot block teardown forever.""" + client = object.__new__(BLEClient) + client._eventThread = MagicMock() + # Force a plain (non-async) mock for the coroutine method so we don't create + # an un-awaited coroutine when close() calls it. + with patch.object(BLEClient, "async_run") as async_run, patch.object( + BLEClient, "_stop_event_loop", new=MagicMock() + ): + client.close() + async_run.assert_called_once() + client._eventThread.join.assert_called_once_with(timeout=BLE_DISCONNECT_TIMEOUT) + + +@pytest.mark.unit +def test_ble_client_async_await_cancels_future_on_timeout(): + """On timeout, async_await must cancel the pending future so a stalled + coroutine is not left running on the event loop.""" + client = object.__new__(BLEClient) + future = MagicMock() + future.result.side_effect = FutureTimeoutError() + with patch.object(BLEClient, "async_run", return_value=future): + with pytest.raises(FutureTimeoutError): + client.async_await("coro", timeout=BLE_DISCONNECT_TIMEOUT) + future.cancel.assert_called_once()