-
Notifications
You must be signed in to change notification settings - Fork 149
Fix memory streams incorrectly raising cancelled when *_nowait()
is called immediately after cancelling send()
/receive()
#729
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
Conversation
…) when calling *_nowait() immediately after cancelling send()/receive() * In the send_nowait() + receive() case, this bug could drop items. * In the send() + receive_nowait() case, this bug could cause send() to raise even though it succeeded.
…alled immediately after cancelling send()/receive() This partially reverts commit 6b0a1f3. Co-authored-by: Alex Grönholm <[email protected]>
This was the first solution that came to my mind, but then I realized it would interact poorly with asyncio's native cancellation. |
could you elaborate on this? i do see now that the branch of |
10bcc86
to
06e38da
Compare
send_nowait()
immediately after cancelling receive()
*_nowait()
immediately after cancelling send()
/receive()
(i just updated this branch to also test for and fix the analogous bug that can occur when |
06e38da
to
2b09585
Compare
*_nowait()
immediately after cancelling send()
/receive()
*_nowait()
is called immediately after cancelling send()
/receive()
# Ignore the immediate cancellation if we already sent the item, so as | ||
# to not indicate failure despite success | ||
if send_event in self._state.waiting_senders: | ||
del self._state.waiting_senders[send_event] | ||
raise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be fine when the cancellation originates from a cancel scope, but not when a task was natively cancelled (Task.cancel()
, because the originator of that cancellation expects the cancellation to be acted on and not ignored). Doing this will lead to hangs.
Closing in favor of #735, as I've come to the conclusion that avoiding the item delivery to a task with a pending cancellation is the only viable solution. |
Changes
Fixes:
MemoryObjectStream can drop items when the receiving end is cancelled #728 (in sense (1) as defined in MemoryObjectStream can drop items when the receiving end is cancelled #728 (comment)). cancellation of
receive()
could drop items, meaning that neither the attempted sender nor the attempted receiver think they are responsible for the item.the analogous issue about cancellation of
send()
. cancellation ofsend()
could result insend()
raising cancelled despite succeeding, meaning that both the attempted sender and the attempted receiver think they are responsible for the item.Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
the you've fulfilled the following conditions (where applicable):
tests/
) added which would fail without your patchdocs/
, in case of behavior changes or newfeatures)
docs/versionhistory.rst
).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.