diff --git a/meshtastic/ble_interface.py b/meshtastic/ble_interface.py index 64a63616f..b66dd8536 100644 --- a/meshtastic/ble_interface.py +++ b/meshtastic/ble_interface.py @@ -56,6 +56,7 @@ def __init__( # pylint: disable=R0917 ) self.should_read = False + self._closing = False logger.debug("Threads starting") self._want_receive = True @@ -250,12 +251,16 @@ def _sendToRadioImpl(self, toRadio) -> None: self.should_read = True def close(self) -> None: + if self._closing: + return + self._closing = True + try: - MeshInterface.close(self) - except Exception as e: - logger.error(f"Error closing mesh interface: {e}") + try: + MeshInterface.close(self) + except Exception as e: + logger.error(f"Error closing mesh interface: {e}") - if self._want_receive: self._want_receive = False # Tell the thread we want it to stop if self._receiveThread: self._receiveThread.join( @@ -263,12 +268,16 @@ def close(self) -> None: ) # If bleak is hung, don't wait for the thread to exit (it is critical we disconnect) self._receiveThread = None - if self.client: - atexit.unregister(self._exit_handler) - self.client.disconnect() - self.client.close() - self.client = None - self._disconnected() # send the disconnected indicator up to clients + if self.client: + self.client.disconnect() + self.client.close() + atexit.unregister(self._exit_handler) + self.client = None + self._disconnected() # send the disconnected indicator up to clients + except BaseException: + # Keep successful closes idempotent, but allow failed cleanup to retry. + self._closing = False + raise class BLEClient: diff --git a/meshtastic/tests/test_ble_interface.py b/meshtastic/tests/test_ble_interface.py index 0b725e65c..9fd7e1d09 100644 --- a/meshtastic/tests/test_ble_interface.py +++ b/meshtastic/tests/test_ble_interface.py @@ -1,5 +1,6 @@ """Meshtastic unit tests for ble_interface.py""" +import atexit from unittest.mock import MagicMock, patch import pytest @@ -67,3 +68,78 @@ 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_close_reentrant_does_not_deadlock() -> None: + """close() must not deadlock when disconnect callback re-enters close().""" + iface = object.__new__(BLEInterface) + iface._closing = False + iface._want_receive = False + iface._receiveThread = None + + disconnect_count = 0 + + def fake_disconnect(): + nonlocal disconnect_count + disconnect_count += 1 + iface.close() # re-entrant call — should return immediately + + mock_client = MagicMock() + mock_client.disconnect.side_effect = fake_disconnect + iface.client = mock_client + iface._exit_handler = lambda: None + + with patch("meshtastic.mesh_interface.MeshInterface.close"), \ + patch.object(iface, "_disconnected"): + iface.close() + iface.close() # A completed close must also remain idempotent. + + assert disconnect_count == 1, "disconnect() should be called exactly once" + assert iface.client is None, "client should be cleaned up" + + +@pytest.mark.unit +@pytest.mark.parametrize("failure", ["join", "disconnect", "close", "notification"]) +def test_ble_close_can_retry_after_cleanup_failure(failure) -> None: + """A failed cleanup must not permanently disable close().""" + iface = object.__new__(BLEInterface) + iface._closing = False + iface._want_receive = True + receive_thread = MagicMock() + iface._receiveThread = receive_thread + client = MagicMock() + iface.client = client + iface._exit_handler = lambda: None + notification = MagicMock() + iface._disconnected = notification + failing_step = { + "join": receive_thread.join, + "disconnect": client.disconnect, + "close": client.close, + "notification": notification, + }[failure] + failing_step.side_effect = RuntimeError("cleanup failed") + + with patch("meshtastic.mesh_interface.MeshInterface.close"), \ + patch("meshtastic.ble_interface.atexit.unregister") as unregister: + with pytest.raises(RuntimeError, match="cleanup failed"): + iface.close() + + assert iface._closing is False + if failure != "notification": + assert iface.client is client + unregister.assert_not_called() + + failing_step.side_effect = None + iface.close() + iface.close() + + assert iface._closing is True + assert iface.client is None + assert iface._receiveThread is None + assert iface._want_receive is False + assert failing_step.call_count == 2 + unregister.assert_called_once_with(iface._exit_handler) + client.close.assert_called() + notification.assert_called()