-
Notifications
You must be signed in to change notification settings - Fork 305
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.
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
Fix DBus connection leak #1699
base: develop
Are you sure you want to change the base?
Fix DBus connection leak #1699
Conversation
Thanks for the fix. I wonder if we could improve this even more with a |
You mean something like this? My python skills are not that great, so it could be nothing. And I'm not sure if I understand the @asynccontextmanager
async def _connect_bus(self):
self._bus = await MessageBus(
bus_type=BusType.SYSTEM,
negotiate_unix_fd=True,
auth=get_dbus_authenticator(),
).connect()
try:
yield
finally:
self._cleanup_all()
def connect():
# ...rest of the code...
async with async_timeout(timeout), AsyncExitStack() as stack:
while True:
# Each BLE connection session needs a new D-Bus connection to avoid a
# BlueZ quirk where notifications are automatically enabled on reconnect.
await stack.enter_async_context(self._connect_bus()) Also I'm unsure if this means that in some places the |
Sort of. But |
Ah so it should be more something like this (I left out most of the irrelevant code): def connect():
async with async_timeout(timeout), AsyncExitStack() as stack:
while True:
self._bus = await MessageBus(
bus_type=BusType.SYSTEM,
negotiate_unix_fd=True,
auth=get_dbus_authenticator(),
).connect()
stack.callback(self._cleanup_all)
try:
try:
await self.get_services(
dangerous_use_bleak_cache=dangerous_use_bleak_cache
)
stack.pop_all() # added line
return True
except: BaseException:
except BaseException:
local_disconnect_monitor_event.set()
# self._cleanup_all() line can be removed
raise |
We should be able to get rid of both try/except statements. And at the very end, we need to call |
2615c5e
to
175e660
Compare
I committed some changes in which I used the |
Moved the try (in bluezdbus/client.py) in order to include the call to the add_device_watcher function, since it is able to throw an error when the device does not exists in bluez anymore. This causes the just opened dbus connection to not be properly closed.
fixes #1698
Pull Request Guidelines for Bleak
Before you submit a pull request, check that it meets these guidelines:
CHANGELOG.rst
, describing your changes as is specified by theguidelines in that document.
different problems and you are committing them in the same PR. In that case,
consider making several PRs instead.
AUTHORS.rst
file!