Fix FileResponse doing blocking I/O in the event loop #8012
Conversation
The call to os.stat was run in the executor but the call to `is_file()` was not. Additionally this resulted in two syscalls to `stat` since both `is_file` and `stat` will call `stat` under the hood. The code has been refactored to ensure there is only one `stat` syscall and it happens in the executor
bdraco
added
backport-3.10
psf-chronographer
bot
added
the
bot:chronographer:provided
|
There is also a race here since nothing guarantees that the file can't change between the time we stat it and the time we open it Fixing that is a lot harder though since we should I opened an issue for that so we don't forget about it, but its such a rare case that I don't think we need to prioritize fixing it |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8012 +/- ##
=======================================
Coverage 97.46% 97.46%
=======================================
Files 107 107
Lines 32526 32529 +3
Branches 3791 3790 -1
=======================================
+ Hits 31700 31703 +3
Misses 624 624
Partials 202 202
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
local testing looks good |
|
looks good on a second production system. doing a |
|
looks good |
Backport to 3.9: 💚 backport PR created✅ Backport PR branch: Backported as #8015 🤖 @patchback |
(cherry picked from commit 5f699bb)
Backport to 3.10: 💚 backport PR created✅ Backport PR branch: Backported as #8016 🤖 @patchback |
(cherry picked from commit 5f699bb)
…O in the event loop (#8016) Co-authored-by: J. Nick Koston <nick@koston.org>
… in the event loop (#8015) Co-authored-by: J. Nick Koston <nick@koston.org>
What do these changes do?
Fix
FileResponsedoing blocking I/O in the event loopThe call to
os.statwas run in the executor but the call tois_file()was not. Additionally this resulted in two syscalls tostatsince bothis_fileandstatwill callstatunder the hood. The code has been refactored to ensure there is only onestatsyscall and it happens in the executorAre there changes in behavior for the user?
No
Related issue number
discovered while investigating #8011 but not does fix the issue reported
Checklist
CONTRIBUTORS.txtCHANGESfolder<issue_id>.<type>for example (588.bugfix)issue_idchange it to the pr id after creating the pr.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.