Skip to content

gh-105331: Fix asyncio.sleep() bug #105641

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

Merged
kumaraditya303 merged 14 commits into from
Jun 12, 2023
Merged

gh-105331: Fix asyncio.sleep() bug #105641

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f2008da
gh-105331: Fix asyncio.sleep() bug
weijay0804 Jun 11, 2023
cb1492f
📜🤖 Added by blurb_it.
blurb-it[bot] Jun 11, 2023
ca806d9
run make patchcheck to fix Azure Pipelines PR problem
weijay0804 Jun 11, 2023
588bbbd
gh-105331: move import math to the toplevel
weijay0804 Jun 11, 2023
1c17886
gh-105331: Improve the content description of the error message
weijay0804 Jun 11, 2023
9bd7ced
gh-105331: Improve description to more detailed
weijay0804 Jun 11, 2023
cc03137
gh-105331: Fix the indentation change
weijay0804 Jun 12, 2023
4a08061
gh-105331: Add test for asyncio.sleep() delay is nan
weijay0804 Jun 12, 2023
11b9d7e
gh-105331: Move check delay is nan section after delay <= 0 section
weijay0804 Jun 12, 2023
7c3f3eb
gh-105331: Add description for versionchanged
weijay0804 Jun 12, 2023
730ee4e
Update Misc/NEWS.d/next/Core and Builtins/2023-06-11-09-14-30.gh-issu…
weijay0804 Jun 12, 2023
49c55c6
minor tweaks
kumaraditya303 Jun 12, 2023
c1712ad
refer to `nan` not `math.nan`
kumaraditya303 Jun 12, 2023
5c840c4
fix news
kumaraditya303 Jun 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions Lib/asyncio/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,12 @@ def __sleep0():

async def sleep(delay, result=None):
"""Coroutine that completes after a given time (in seconds)."""

# check delay value is not nan
import math

@gvanrossum gvanrossum Jun 11, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just move this import to the toplevel, sorted between the rest of the imports.

if math.isnan(delay):

@kumaraditya303 kumaraditya303 Jun 12, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this check after delay <= 0 check to not slow down the common case of sleep(0).

raise ValueError("Invalid value NaN (not a number)")

@gvanrossum gvanrossum Jun 11, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:

Suggested change
raise ValueError("Invalid value NaN (not a number)")
raise ValueError("Invalid delay: NaN (not a number)")


if delay <= 0:
await __sleep0()
return result
Expand Down Expand Up @@ -944,21 +950,21 @@ def callback():
def create_eager_task_factory(custom_task_constructor):
"""Create a function suitable for use as a task factory on an event-loop.

Example usage:
Example usage:

@gvanrossum gvanrossum Jun 11, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the indentation change you (accidentally?) applied to much of this docstring. (Maybe your editor inserted tabs instead of spaces?)


loop.set_task_factory(
asyncio.create_eager_task_factory(my_task_constructor))
loop.set_task_factory(
asyncio.create_eager_task_factory(my_task_constructor))

Now, tasks created will be started immediately (rather than being first
scheduled to an event loop). The constructor argument can be any callable
that returns a Task-compatible object and has a signature compatible
with `Task.__init__`; it must have the `eager_start` keyword argument.
Now, tasks created will be started immediately (rather than being first
scheduled to an event loop). The constructor argument can be any callable
that returns a Task-compatible object and has a signature compatible
with `Task.__init__`; it must have the `eager_start` keyword argument.

Most applications will use `Task` for `custom_task_constructor` and in
Most applications will use `Task` for `custom_task_constructor` and in
this case there's no need to call `create_eager_task_factory()`
directly. Instead the global `eager_task_factory` instance can be
used. E.g. `loop.set_task_factory(asyncio.eager_task_factory)`.
"""
"""

def factory(loop, coro, *, name=None, context=None):
return custom_task_constructor(
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2023-06-11-09-14-30.gh-issue-105331.nlZvoW.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fix asyncio.sleep(float('nan')) does not raise ValueErro problem

@gvanrossum gvanrossum Jun 11, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try harder here. The text requires proper punctuation, capitalization, markup, and it should describe the change in behavior rather than just stating "fix the problem".