Skip to content
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

FlattenUnordered: improve wakers behavior #2566

Merged

Conversation

olegnn
Copy link
Contributor

@olegnn olegnn commented Feb 6, 2022

Fixes a rare case when the same waker (for stream or inner streams) was called twice and more times at the same time, and the stream was polled before the wakers ended their waking process, as this may lead to incorrect access to the UnsafeCell from some of the waking wakers.

@olegnn olegnn requested a review from taiki-e as a code owner Feb 6, 2022
@olegnn olegnn force-pushed the flatten-unordered-improve-wakers-behavior branch from c74e9a0 to 212107c Compare Feb 6, 2022
@taiki-e
Copy link
Member

taiki-e commented Feb 6, 2022

Thanks!

this may lead to incorrect access to the UnsafeCell from some of the waking wakers.

Does this mean it will cause a data race?

@taiki-e taiki-e added 0.3-backport: pending A-stream labels Feb 6, 2022
@olegnn
Copy link
Contributor Author

olegnn commented Feb 6, 2022

Does this mean it will cause a data race?

Well, the UnsafeCell may be potentially modified at the moment while it's being read, so yes.
However, to make it happen, we need the following sequence:

  • Some waker (for ex. for inner streams) was called without delay several times from different threads
  • We started to poll the stream before the last waker ends its waking process
  • Some of the wakers reads potentially bad waker from the unsafe cell

taiki-e
taiki-e approved these changes Feb 6, 2022
@taiki-e taiki-e merged commit 4aaa2d9 into rust-lang:master Feb 6, 2022
23 checks passed
@taiki-e taiki-e mentioned this pull request Feb 6, 2022
@taiki-e taiki-e added 0.3-backport: completed and removed 0.3-backport: pending labels Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants