gh-105331: Fix asyncio.sleep() bug #105513
Conversation
weijay0804
requested review from
1st1,
asvetlov,
gvanrossum,
kumaraditya303 and
willingc
as code owners
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
| raise TypeError(f"{type(delay)} object cannot be interpreted as an integer") | ||
|
|
||
| # According to Modules/timemodule.c | ||
| if (delay > sys.maxsize / SEC_TO_NS): |
There was a problem hiding this comment.
It is incorrect because of 32-bit platform (and that's why asyncio test failing on Windows 32-bit).
sys.maxsize on 32-bit platform equal to 2 ** 31 -1.
So, in that case sys.maxsize / SEC_TO_NS is equal to 2.147483647
There was a problem hiding this comment.
Oh!
I forgot about this matter.
In timemodule.c, INT64_MAX is used and is not subject to the limitation of 32-bit.
If I write it as INT64_MAX = 2**64 - 1, how would it be?
if (delay > INT64_MAX / SEC_TO_NS)
There was a problem hiding this comment.
Oh! I forgot about this matter. In
timemodule.c, INT64_MAX is used and is not subject to the limitation of 32-bit. If I write it asINT64_MAX = 2**64 - 1, how would it be?
if (delay > INT64_MAX / SEC_TO_NS)
I think it's correct solution, but I'm not expert in this area. Let's wait for other opinions
| if (delay < 0): | ||
| raise ValueError("sleep length must be non-negative") | ||
|
|
||
| if (math.isnan(delay)): |
There was a problem hiding this comment.
Also, seems there need to add check that delay is finite number. Use something like if not math.isfinite(delay): ... for resolve that.
There was a problem hiding this comment.
This appears to be more accurate.
Thank you!
|
This PR has some scope creep compared to the issue it purports to fix -- it changes the behavior if the delay is negative. While this was discussed in the issue, no conclusion was drawn (there were conflicting opinions). If it was just the NaN check I'd approve it after fixing the grammar, punctuation and markup in the NEWS file, but as it is, I can't. |
fix asyncio.sleep(float('nan')) does not raise ValueError