Skip to content

fix(ble): don't hang on a stalled bleak disconnect during teardown #967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lalonggone wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(ble): don't hang on a stalled bleak disconnect during teardown #967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b6db75
fix(ble): don't hang on a stalled bleak disconnect during teardown
lalonggone Aug 7, 2026
2a47f6e
fix(ble): cancel the disconnect future when it times out
lalonggone Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions meshtastic/ble_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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__)


Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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}")
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def read_gatt_char(self, *args, **kwargs): # pylint: disable=C0116
return self.async_await(self.bleak_client.read_gatt_char(*args, **kwargs))
Expand All @@ -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)
Comment on lines +335 to +337

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files:"
git ls-files | rg '(^|/)meshtastic/ble_interface\.py$|pyproject|requirements|poetry|setup' || true

echo
echo "ble_interface outline:"
ast-grep outline meshtastic/ble_interface.py --view compact || true

echo
echo "Relevant function excerpts:"
grep -n "def connect\|def disconnect\|def close\|_eventThread\|disconnected_callback\|BLE_DISCONNECT_TIMEOUT" meshtastic/ble_interface.py || true

echo
echo "Line ranges:"
nl -ba meshtastic/ble_interface.py | sed -n '300,380p'
nl -ba meshtastic/ble_interface.py | sed -n '1,120p'
nl -ba meshtastic/ble_interface.py | sed -n '120,220p'

Repository: meshtastic/python

Length of output: 1112

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Line 1-130:"
cat -n meshtastic/ble_interface.py | sed -n '1,130p'

echo
echo "Line 180-350:"
cat -n meshtastic/ble_interface.py | sed -n '180,350p'

echo
echo "Bleak versions:"
rg -n "bleak|Bleak|Dis" poetry.lock pyproject.toml || true

Repository: meshtastic/python

Length of output: 14707

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "BLEClient async helpers:"
cat -n meshtastic/ble_interface.py | sed -n '345,430p'

echo
echo "asyncio docs for Thread.join current thread in Python:"
python3 - <<'PY'
import threading
import asyncio

eventLoop = asyncio.new_event_loop()
asyncio.set_event_loop(eventLoop)
eventThread = threading.Thread(target=eventLoop.run_forever)
eventThread.start()

thread = threading.current_thread()
try:
    thread.join(timeout=0.001)
except Exception as exc:
    print(type(exc).__name__, str(exc))
else:
    print("no exception")
finally:
    eventLoop.call_soon_threadsafe(eventLoop.stop)
    eventThread.join(1)
    eventLoop.close()
PY

echo
echo "Bleak 0.22.3 callback scheduling references available:"
python3 - <<'PY'
import urllib.request
urls = [
    "https://raw.githubusercontent.com/hbldh/bleak/v0.22.3/bleak/__init__.py",
    "https://raw.githubusercontent.com/hbldh/bleak/v1.1.1/bleak/__init__.py",
]
for url in urls:
    print("URL", url)
    try:
        text = urllib.request.urlopen(url, timeout=20).read().decode()
        for i, line in enumerate(text.splitlines(), 1):
            if "disconnected_callback" in line or "run_coroutine_threadsafe" in line or "disconnection_callback" in line:
                lo=max(1,i-3); hi=min(len(text.splitlines()), i+5)
                lines=text.splitlines()
                print(f"line {i}: {lines[i-1]}")
                print("\n".join([f"{j}: {lines[j-1]}" for j in range(lo, hi+1) if lines[j-1].strip()]))
                print()
    except Exception as exc:
        print("ERROR", exc)
PY

echo
echo "Bleak 1.1.1 relevant source snippets:"
tmp="$(mktemp -d)"
pip install bleach==1.1.1 -q
python3 - <<'PY'
import inspect
import pkgutil
import uuid
mod = __import__('bleak', fromlist=['__version__'])
print("bleak.__version__", getattr(mod, "__version__", "unknown"))
for name in ["BleakClient"]:
    obj = getattr(mod, name, None)
    if obj:
        print(name)
        for src_line in inspect.getsourcelines(obj)[0]:
            if "disconnected_callback" in src_line or "run_coroutine_threadsafe" in src_line:
                print(src_line.rstrip())
PY
rm -rf "$tmp"

Repository: meshtastic/python

Length of output: 3067

🌐 Web query:

Bleak 0.22.3 BleakClient disconnected_callback run_coroutine_threadsafe source

💡 Result:

In Bleak 0.22.3, the disconnected_callback provided to the BleakClient constructor is scheduled in the asyncio event loop when a device disconnects [1][2]. Because operating system-level disconnection events often occur on background threads, Bleak handles the synchronization internally [3]. Key technical details regarding this mechanism include: 1. Thread Safety: When an OS-level disconnect event is received on a background thread, Bleak uses methods like call_soon_threadsafe() internally to ensure that the user's provided disconnected_callback is executed on the correct asyncio event loop thread [4][5][3]. Users generally do not need to manually call run_coroutine_threadsafe when passing a standard callable as the disconnected_callback. 2. Callback Requirements: The disconnected_callback must be a synchronous callable that accepts one argument: the BleakClient instance itself [1][6]. If you attempt to provide an async function (coroutine) as the callback, it will not be awaited automatically and will likely result in a RuntimeWarning [3]. 3. Version 0.22.3 Fixes: Version 0.22.3 specifically addressed an issue where the object passed to the disconnected_callback was not correctly identified as the BleakClient instance in certain backends, ensuring the callback receives the expected client object [4][5]. If you need to perform asynchronous operations (such as awaiting a coroutine) within your disconnect logic, you should wrap the asynchronous task inside your synchronous disconnected_callback using asyncio.create_task() or, if calling from outside the event loop, asyncio.run_coroutine_threadsafe() to bridge the threads safely [7][3].

Citations:

🌐 Web query:

Bleak 0.22.3 BleakClient __init__ disconnected_callback source call_soon_threadsafe asyncio

💡 Result:

In Bleak version 0.22.3, the disconnected_callback mechanism is designed to bridge events from backend threads to the asyncio event loop [1][2]. When you provide a disconnected_callback to the BleakClient constructor, Bleak wraps it using functools.partial to ensure the callback receives the BleakClient instance as its first argument [3]. Because the underlying OS-level Bluetooth events often occur on background threads, Bleak must perform thread-safe scheduling to execute your callback within the proper asyncio event loop [1][2][4]. Specifically, in version 0.22.3, a fix was implemented to ensure that the object passed to the disconnected_callback is the BleakClient instance itself, rather than the internal backend object [1][2]. Key technical points regarding this implementation: - Thread-Safety: Bleak uses call_soon_threadsafe (or equivalent mechanisms in backend-specific code) to schedule the callback execution on the main event loop from the background thread where the disconnection event originated [1][2][4]. - Callback Signature: The disconnected_callback must be a synchronous callable that accepts a single argument, which is the BleakClient object [5][6][7]. It cannot be an async function (coroutine), as the executor will not await it, leading to a RuntimeWarning [4]. - Implementation detail: The wrapper logic in bleak/init.py creates a partial function: self._backend = PlatformBleakClient(..., disconnected_callback=(None if disconnected_callback is None else functools.partial(disconnected_callback, self)),...) [3]. This ensures that when the backend triggers the callback, the user's function is invoked with the expected client reference [1][2].

Citations:

🌐 Web query:

Bleak backend disconnect callback call_soon call_soon_threadsafe source 0.22.3

💡 Result:

In Bleak, the disconnected_callback is scheduled within the asyncio event loop to ensure it runs on the main event loop thread, even when the underlying OS-level disconnection signal arrives on a background thread [1]. When an unsolicited disconnection occurs, the Bleak backend uses a mechanism to safely invoke the user-provided callback [1]. Specifically, Bleak uses call_soon_threadsafe (or sometimes a wrapper that ensures proper scheduling) to bridge the gap between the background thread (often where native Bluetooth OS APIs report events) and the asyncio event loop [2][1]. This ensures that your callback logic is executed in a thread-safe manner within the event loop's context [1]. Key points regarding this mechanism in Bleak (including version 0.22.3 and surrounding releases): 1. Implementation: When the operating system notifies the backend of a disconnect, Bleak's internal delegate or monitor detects this event [3]. It then schedules the execution of the user's callback using loop.call_soon_threadsafe to ensure it is handled correctly by the asyncio loop [2][1]. 2. Callback Requirements: The callback function must be a standard, synchronous callable that accepts the client object as its sole argument [4][5][1]. Because Bleak schedules this callback in the event loop, you cannot pass an async function (coroutine) directly; attempting to do so will result in a RuntimeWarning because the coroutine will not be awaited [1]. 3. Purpose of Thread Safety: Since native Bluetooth stacks (such as CoreBluetooth on macOS or others) often operate on their own threads, call_soon_threadsafe is critical for preventing race conditions and maintaining the integrity of the asyncio event loop [2][1]. If you are experiencing issues with the callback not firing or errors during the callback execution, ensure your callback is not an async function and does not perform blocking operations directly, as it runs on the event loop thread [1]. If you need to perform asynchronous work inside the callback, you should use loop.call_soon or create a task from within the callback to schedule that work properly.

Citations:

Do not block or join BLEClient._eventThread on the BLE event-loop thread.

BLEInterface.connect passes lambda _: self.close() as the Bleak disconnected_callback. Unsolicited disconnects are delivered through Bleak’s callback path, so self.client.close() can run on BLEClient._eventThread. The subsequent self._eventThread.join(...) then fails with RuntimeError: cannot join current thread, and cleanup can return before self.client = None and _disconnected() run. Move the teardown path to a non-loop thread, or make disconnect() / close() detect the owner thread and avoid synchronous waiting and self-joining. Add coverage for this disconnect-callback path.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@meshtastic/ble_interface.py` around lines 335 - 337, The BLE disconnect
teardown must not join BLEClient._eventThread when invoked by its own callback.
Update BLEInterface.disconnect/close and the self._eventThread.join path to
detect the event-loop owner thread and perform teardown asynchronously or skip
synchronous waiting, while preserving normal bounded joining from other threads;
add coverage for the disconnected_callback path.


def __enter__(self):
return self
Expand All @@ -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

Expand Down
67 changes: 65 additions & 2 deletions meshtastic/tests/test_ble_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()