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

Characteristic notification handler goroutine leak #260

Open
Elara6331 opened this issue Apr 2, 2024 · 2 comments · May be fixed by #262
Open

Characteristic notification handler goroutine leak #260

Elara6331 opened this issue Apr 2, 2024 · 2 comments · May be fixed by #262

Comments

@Elara6331
Copy link
Contributor

Elara6331 commented Apr 2, 2024

When notifications are enabled on a DeviceCharacteristic on Linux, it starts a new goroutine listening on a channel for DBus signals, but when a device disconnects (or when EnableNotifications(nil) is called), the c.property channel is never closed, so the goroutine will keep waiting for a signal on its property channel.

In the case of EnableNotifications(nil), the goroutine will sleep forever and the signal will never come due to the call to RemoveSignal here:

c.adapter.bus.RemoveSignal(c.property)

However, when a device disconnects, RemoveSignal is never called, so when a new notification handler is added to that characteristic after the device reconnects, the MatchSignal is re-added, so the old goroutine starts receiving signals again and executes its old handler, causing duplicated events and other undesirable issues.

@jasday
Copy link

jasday commented Apr 2, 2024

I saw similar behavior in v0.8.0, along with the memory leak in my issue here #252.

I'm no longer seeing the memory leak as mentioned in my issue, but I am seeing a leak in v0.9.0 such as the one you mentioned here when connecting to a device and enabling notifications, along with undesirable events if EnableNotificaitons(nil) is not called

@Elara6331
Copy link
Contributor Author

Fixing the EnableNotifications(nil) leak is easy enough. Adding a close(c.property) before c.property = nil should fix it. The disconnect issue is more difficult to fix because you'd have to detect a device disconnect and release all the handler goroutines.

@Elara6331 Elara6331 linked a pull request Apr 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants