-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
Keep track of TimerHandles independent from event loop #109022
Conversation
homeassistant/helpers/event.py
Outdated
return hass.loop.call_at(loop_time, _run_async_call_action, hass, job).cancel | ||
handle = hass.loop.call_at(loop_time, _run_async_call_action, hass, job) | ||
if job.cancel_on_shutdown: | ||
hass.async_track_timer_handle(handle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upside to doing this here is that its not doing for all handles which makes it cheaper. It still means uvloop would fail in tests since async_fire_time_changed
in the tests helper needs access to ._scheduled
. But since its only in tests, maybe we can figure out how to access whats scheduled based on if its asyncio or uvloop in the test code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh indeed
I'm not sure the broadcast issue is sorted out MagicStack/uvloop#540 |
Nice. Any chance you can work on which change in uvloop made it work so we can document it in that issue and ask the op to close it ? |
Yes, I'll try it |
Okay, my bad... With |
MagicStack/uvloop#592 should fix the broadcast issue. Lets see if they are happy with it. |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Do we want to continue with this? |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
It looks like uvloop hasn't merged anything in 8 months and needs some cython 3 fixes as well. It would be nice to use it but its not looking like its being maintained these days. |
Proposed change
Currently we're relaying on the event loop to keep track / get the scheduled TimerHandles. This could / is a problem if we consider switching to uvloop, besides the
_scheduled
property of theasyncio
event loop is a protected property, so this may break anytime (what I don't expect to)So this PR proposes to keep track of the TimeHandles, they need to be cancelled at shutdown, on our own.
I use a
WeakSet
here, so we don't have to remove the reference from the set ourselves after a TimeHandle has ended.I tried also a different solution within
uvloop
(create a function to return the scheduled TimeHandlers), but lead me to other issues e.g._args
orargs
weren't available for theuvloop
TimeHandlers.I may have forgotten something, or it may not be the ideal solution, so I'll leave this here for comments.
With this solution the implementation ofuvloop
would be quite simple, the problem with<broadcast>
described in #93173 seems to be solved. I have tried this with a small test and can send packets to the<broadcast>
address without any problemsThe
<broadcast>
issue within uvloop mentioned in #93173 should be solved by MagicStack/uvloop#592.Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: