Skip to content
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

Memory leaks through missing callback deregistration in Bluez #89

Closed
JoeHut opened this issue Jul 16, 2019 · 2 comments · Fixed by #90 or workaroundgmbh/bleak#2
Closed

Memory leaks through missing callback deregistration in Bluez #89

JoeHut opened this issue Jul 16, 2019 · 2 comments · Fixed by #90 or workaroundgmbh/bleak#2
Assignees
Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working

Comments

@JoeHut
Copy link
Contributor

JoeHut commented Jul 16, 2019

  • bleak version: master
  • Python version: 3.7.3
  • Operating System: Linux
  • Bluez version: 5.50

Description

In my use case I have a long running application that is looking for devices with certain properties (like a name prefix, RSSI etc.) and connects to them. To accomplish this I have an infinite loop that calls discover and checks the list of devices until the proper one is available. Something like:

while True:
    devices = await discover(timeout=1)
    for d in devices:
        if d.name.startswith("PREFIX"):
            return d

I observed that if it is running for a while, the discover takes not only around a second but up to multiple minutes.

Explanation

The issue is that the DBus callbacks in discovery() for InterfacesAdded, InterfacesRemoved and PropertiesChanged are never deleted again. Therefore they pile up and the parse_msg callback is called thousands of times after a couple of minutes.

The issue is therefore similar to #83 where the notifications are sent multiple times after a reconnection. I think it makes sense to go through all Bluez callback registrations and make sure they don't leak.

@JoeHut
Copy link
Contributor Author

JoeHut commented Jul 16, 2019

@hbldh I will add a PR for this. Please let me know if there are other ways to handle those callback de-registrations. I am not an expert for DBus/Bluez

JoeHut pushed a commit to workaroundgmbh/bleak that referenced this issue Jul 16, 2019
If the rules are only added to Bluez via DBus but never removed again,
the calls pile up if discover() is called multiple times in one session.
This is especially harmful for long-running applications that connect
and disconnect to the same or different servers over and over. The time
to await discovery() becomes very long (up to minutes) even with smaller
timeouts.

Make sure the callbacks registered are cleaned up before discover() is
left.

Fixes hbldh#89
JoeHut pushed a commit to workaroundgmbh/bleak that referenced this issue Jul 16, 2019
If the rules are only added to Bluez via DBus but never removed again,
the calls pile up if discover() is called multiple times in one session.
This is especially harmful for long-running applications that connect
and disconnect to the same or different servers over and over. The time
to await discovery() becomes very long (up to minutes) even with smaller
timeouts.

Make sure the callbacks registered are cleaned up before discover() is
left.

Fixes hbldh#89
@JoeHut JoeHut reopened this Jul 16, 2019
@hbldh hbldh self-assigned this Jul 16, 2019
@hbldh hbldh added Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working labels Jul 16, 2019
@hbldh
Copy link
Owner

hbldh commented Jul 16, 2019

This is a very good catch. Very bad thing to miss this. Will integrate it asap.

@hbldh hbldh closed this as completed in #90 Aug 2, 2019
hbldh added a commit that referenced this issue Aug 2, 2019
macOS support added (thanks to @kevincar)
Merged #80: macOS development
Merged #90 which fixed #89: Leaking callbacks in BlueZ
Merged #92 which fixed #91, Prevent leaking of DBus connections on discovery
Merged #96: Regex patterns
Merged #86 which fixed #83 and #82
Recovered old .NET discovery method to try for #95
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working
Projects
None yet
2 participants