fix(transport): close all BLE notification-watcher leak paths#138
Merged
g4bri3lDev merged 1 commit intoJul 17, 2026
Merged
Conversation
A BlueZ notification watcher registered by start_notify() is only released on disconnect. Three paths could leave a connected+subscribed client without a guaranteed disconnect, leaking a watcher that then delivers stray/duplicate frames to later connections (observed as a monotonically rising "BlueZ device watcher count" across failed probes, and duplicate rx on a live connection while a prior probe's watcher lingered). Path 1 — OpenDisplayDevice.__aenter__ ran authenticate/interrogate after connect() with no guard. Any raise there (AuthenticationRequiredError on a keyless probe, or CancelledError from an outer asyncio.timeout in the HA config-flow probe) propagates out of __aenter__, so __aexit__ never runs and the live connection leaks. Wrap the post-connect steps and disconnect() on BaseException (CancelledError is not an Exception). Paths 2 & 3 — _clear_cache_and_drop() and disconnect() null the client even when disconnect() raised, stranding the watcher. Add _stop_notifications() and call it before every disconnect so the watcher is released even if the subsequent ACL disconnect is flaky; reset _notification_characteristic alongside _client. Adds regression tests for all three paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
Author
|
FYI @jonasniesner this fixes the encryption bug with the Solum tags. It is due to "watcher" leaks due to BLE connections not properly unwinding themselves on error |
g4bri3lDev
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Close three BLE notification-watcher leak paths on the BlueZ backend. A watcher registered by
start_notify()is only released on disconnect; any path that reaches a connected+subscribed client without a guaranteed disconnect leaks the watcher, which then delivers stray/duplicate notifications to a later connection (observed as a monotonically rising BlueZ device-watcher count across failed probes, and duplicate rx on a live connection).The three paths
OpenDisplayDevice.__aenter__ranauthenticate()/interrogate()afterconnect()with no guard. Any raise there —AuthenticationRequiredErroron a keyless probe, orCancelledErrorfrom an outerasyncio.timeout()(e.g. an HA config-flow probe) — propagates out of__aenter__, so__aexit__never runs and the live connection leaks. Now wrapped: onBaseExceptionwe disconnect and re-raise (BaseException, notException, becauseCancelledErroris not anException)._clear_cache_and_drop()and 3.disconnect()nulled the client even whendisconnect()raised, stranding the watcher. Added_stop_notifications()(best-effort, never raises) and call it before every disconnect, so the watcher is released even if the subsequent ACL disconnect is flaky;_notification_characteristicis reset alongside_client.Tests
Adds
tests/unit/test_connection_watcher_leak.py— one regression test per leak path (6 cases total).Notes
_LOGGERat existing levels — no new instance state.