You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
asyncio queue: get/put will not awaken next if interrupted #108042
I am confident this is a bug in CPython, not a bug in a third-party project
I have searched the CPython issue tracker,
and am confident this bug has not been reported before
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Output from running 'python -VV' on the command line:
Python 3.11.4 (main, Jun 9 2023, 07:59:55) [GCC 12.3.0]
A clear and concise description of the bug:
If the current putter has an exception thrown (such as when the task awaiting put() has an exception thrown) and the queue isn't full (which can happen if another task put()s before the current task is rewoken), then the queue must wake up the next putter, because otherwise nothing else will wake up the next putter. This line is responsible for this:
The code is correct; it's the documentation that's lacking. The condition in question catches a mismatch between the awaited result and the future's result. Imagine a chain of awaited futures, ex. "await Fut(await Fut(await Fut(await Fut())))". Cancelling any future propagates the error down the chain of await, cancelling each future, and then back up, returning CancelledError. Except of course, when the future already has a result set. This example best illustrates the situation:
The exact same situation can happen if get_nowait() sets a result on a putter, but then another task (indirectly) cancels the very same putter - a CancelledError is raised, but putter.cancelled() is not true.
task1: put(1)
task2: put(2)
task3: get_nowait() -> task1.putter.set_result()
task4: cancel task1
task1: except CancelledError but not putter.cancelled() -> wake next putter
If the converse occurs it isn't necessary to wake the next putter - if the putter is first cancelled, then get_nowait will simply skip it.
Please change the labels. This isn't a bug. This may not be a documentation issue - the public documentation is correct. Possibly some internal comments could use clarification. I'm not sure how obvious this is to others. Feel free to close if you think there's no need to improve the comments.
I'm a bit unsure about what you're getting at. If the docs are right and there's no bug, maybe we could just throw in some comments in the code to make it clear for everyone else?
OK but I don't want to make it too verbose, ie not a copy-and-paste of my first comment. How's this:
# We were woken up by put_nowait(), but can't take the
# call. Wake up the next in line.
#
# This can happen when put_nowait() sets a result on the
# getter, but then another task (indirectly) cancels the
# very same getter - a CancelledError is raised, but since
# a result is already set getter.cancelled() cannot be
# true.
#
# If the converse occurs it isn't necessary to wake the
# next getter - if the getter is first cancelled, then
# put_nowait() will simply skip it.
#
# See github/cpython/issues/108042 for more information.
As an improvement over:
# We were woken up by put_nowait(), but can't take
# the call. Wake up the next in line.
I'm not sure we need to add such a verbose comment. Sure, the code is subtle. But so is a lot of asyncio internals. I'm closing this as won't fix. (You can still submit a PR with a better comment, but please a lot shorter. Enough that your former self wouldn't have filed a bug. :-)
Activity
orbisvicis commentedon Aug 16, 2023
The code is correct; it's the documentation that's lacking. The condition in question catches a mismatch between the awaited result and the future's result. Imagine a chain of awaited futures, ex. "await Fut(await Fut(await Fut(await Fut())))". Cancelling any future propagates the error down the chain of await, cancelling each future, and then back up, returning CancelledError. Except of course, when the future already has a result set. This example best illustrates the situation:
The exact same situation can happen if
get_nowait()sets a result on a putter, but then another task (indirectly) cancels the very same putter - aCancelledErroris raised, butputter.cancelled()is not true.If the converse occurs it isn't necessary to wake the next putter - if the putter is first cancelled, then
get_nowaitwill simply skip it.orbisvicis commentedon Aug 16, 2023
Please change the labels. This isn't a bug. This may not be a documentation issue - the public documentation is correct. Possibly some internal comments could use clarification. I'm not sure how obvious this is to others. Feel free to close if you think there's no need to improve the comments.
Hels15 commentedon Aug 16, 2023
I'm a bit unsure about what you're getting at. If the docs are right and there's no bug, maybe we could just throw in some comments in the code to make it clear for everyone else?
Hels15 commentedon Aug 16, 2023
if so, maybe you can open a pr with the improved comments.
orbisvicis commentedon Aug 16, 2023
OK but I don't want to make it too verbose, ie not a copy-and-paste of my first comment. How's this:
As an improvement over:
gvanrossum commentedon Aug 16, 2023
I'm not sure we need to add such a verbose comment. Sure, the code is subtle. But so is a lot of asyncio internals. I'm closing this as won't fix. (You can still submit a PR with a better comment, but please a lot shorter. Enough that your former self wouldn't have filed a bug. :-)