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
Fixed memory object stream sometimes dropping sent items #735
base: master
Are you sure you want to change the base?
Conversation
Check if the receiving task has a pending cancellation before sending an item. Fixes #728.
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 looks good to me, modulo the single concern about a possible uncancel edge case.
I'm not sure whether that even can happen, but it seems worth checking. If it can, I'd probably just add a code comment + note in the docs and merge anyway.
:return: a list of task info objects | ||
:return: a sequence of task info objects |
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.
If we want this to be a general Sequence
, let's also update the type annotation above and cast below.
while self._state.waiting_receivers: | ||
receive_event, receiver = self._state.waiting_receivers.popitem(last=False) |
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.
I suspect that this can deadlock if the receiving task is cancelled, then send_nowait invoked (removing it from the waiting_receivers), and then uncancelled before the event loop actually kicks the receiving task out of await receive
.
Of course this is a pretty nasty use of the already brittle uncancel semantics, but we probably want a comment here to make the possibility salient to future readers? Or maybe I'm wrong about this.
cancel_scope.cancel() | ||
send.send_nowait("item") | ||
|
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.
from comment above: uncancelling that scope is my maybe-deadlock.
Changes
Fixes #728.
Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
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.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**
as the version.If, say, your patch fixes issue #123, the entry should look like this:
* Fix big bad boo-boo in task groups (#123 <https://github.com/agronholm/anyio/issues/123>_; PR by Yourname)
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.
If possible, use your real name in the changelog entry. If not, use your GitHub
username.