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.

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

Jump to bottom

The disconnect monitor task can get garbage collected before it finishes #1258

Closed
bdraco opened this issue Mar 18, 2023 · 1 comment · Fixed by #1259 or #1265
Closed

The disconnect monitor task can get garbage collected before it finishes #1258

bdraco opened this issue Mar 18, 2023 · 1 comment · Fixed by #1259 or #1265
Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working

Comments

@bdraco
Copy link
Contributor

bdraco commented Mar 18, 2023 •
edited
Loading

bleak/bleak/backends/bluezdbus/client.py

Line 246 in 1b93859

asyncio.ensure_future(

nothing is holding a reference to the task and asyncio tasks are tracked in weakset which means the task can get garbage collected before it finishes

Sorry I didn't point this out before. I only noticed when reviewing the 0.20 changes for HA compatibility

https://docs.python.org/3/library/asyncio-task.html#creating-tasks

From the asyncio tasks section:

Important Save a reference to the result of this function, to avoid a task disappearing mid-execution. The event loop only keeps weak references to tasks. A task that isn’t referenced elsewhere may get garbage collected at any time, even before it’s done. For reliable “fire-and-forget” background tasks, gather them in a collection:
@bdraco bdraco mentioned this issue Mar 18, 2023
Closed
20 tasks
@dlech dlech added bug Something isn't working Backend: BlueZ Issues and PRs relating to the BlueZ backend labels Mar 18, 2023
dlech added a commit that referenced this issue Mar 18, 2023
@dlech
bleak: fix leaking of ensure_future()
598122b 
As noted in the Python docs, a reference to the return value of
`asyncio.ensure_future()` must be held in order to prevent it from
being garbage collected before the task completes.

This applies the recommended fix from the docs of holding the reference
in a global set and then discarding the reference when the task
completes.

Also change `asyncio.ensure_future()` to `asyncio.create_task()` while
we are touching this since the minimum supported Python version is now
3.7.

Fixes: #1258
@dlech dlech mentioned this issue Mar 18, 2023
Merged
dlech added a commit that referenced this issue Mar 20, 2023
@dlech
bleak: fix leaking of ensure_future()
e01ae59 
As noted in the Python docs, a reference to the return value of
`asyncio.ensure_future()` must be held in order to prevent it from
being garbage collected before the task completes.

This applies the recommended fix from the docs of holding the reference
in a global set and then discarding the reference when the task
completes.

Also change `asyncio.ensure_future()` to `asyncio.create_task()` while
we are touching this since the minimum supported Python version is now
3.7.

Fixes: #1258
@bdraco
Copy link
Contributor Author

bdraco commented Mar 20, 2023

Thank you

@dlech dlech mentioned this issue Mar 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

bleak: fix leaking of ensure_future() hbldh/bleak
v0.20.1 hbldh/bleak
2 participants
@bdraco @dlech