Fix BLE close() deadlock from re-entrant disconnect callback

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,25 +251,33 @@ 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(
                     timeout=2
                 )  # 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:

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()