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

Async all the things that were blocking. #90

Merged
merged 7 commits into from
Jun 20, 2024

Conversation

gwww
Copy link
Contributor

@gwww gwww commented Jun 19, 2024

Remove blocking errors that are showing up in Home Assistant. See home-assistant/core#119084

@bdraco
Copy link

bdraco commented Jun 19, 2024

Note that Home Assistant only checks for open calls, once you find one, please make sure all the read/writes that happen after the open call are also happening in the executor since Home Assistant never checks read/write calls and you will likely never find them again otherwise (and the problem will be much harder to find).

@gwww
Copy link
Contributor Author

gwww commented Jun 19, 2024

Is there an way to tell if read/writes are happening? This code is buried deep in the pillow library - hard to know where reads might be happening. Trying to brut force it at the moment looking through code.

@gwww gwww marked this pull request as draft June 19, 2024 16:00
@gwww
Copy link
Contributor Author

gwww commented Jun 19, 2024

Switched to draft PR until can figure out read/writes.

@bdraco
Copy link

bdraco commented Jun 19, 2024

strace is usually the best way to verify if a read or write syscall is happening and in which thread

env_canada/ec_radar.py Outdated Show resolved Hide resolved
@gwww gwww marked this pull request as ready for review June 19, 2024 19:18
@gwww
Copy link
Contributor Author

gwww commented Jun 19, 2024

A few more changes to encapsulate all the PIL calls. While this looks like a big change it is mostly just moving blocks of code around and so that the PIL call could all be put into a single function that is run_in_executor.

@gwww
Copy link
Contributor Author

gwww commented Jun 19, 2024

I've consolidated all the run_in_executor calls down to 2. I'd get rid of the last one but the code complexity is risky for (at least me) to change.

Really, really ready for review! Thanks for your comments @bdraco! Super helpful!

@bdraco
Copy link

bdraco commented Jun 19, 2024

Looks like its async safe now 👍

At least I don't see anything obvious that is a problem, but its always hard to tell without doing the strace to see where the I/O happens if you don't know all the code in the underlying library (PIL), which I don't

@michaeldavie michaeldavie merged commit 9a467c4 into michaeldavie:master Jun 20, 2024
4 checks passed
@michaeldavie
Copy link
Owner

Thanks @gwww!

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 this pull request may close these issues.

None yet

3 participants