-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
WaitCell::wait
does not tolerate spurious polls
#449
Labels
Comments
hawkw
added
kind/bug
Something isn't working
crate/maitake
Related to the `maitake` crate
labels
Jul 22, 2023
hawkw
added a commit
that referenced
this issue
Jul 22, 2023
hawkw
added a commit
that referenced
this issue
Jul 22, 2023
This branch changes the `Wait` future for `maitake::sync::WaitCell` to handle spurious polls correctly. Currently, a `wait_cell::Wait` future assumes that if it's ever polled a second time, that means its waker was woken. However, there might be other reasons that a stack of futures containing a `Wait` is polled again, and the `Wait` future will incorrectly complete immediately in that case. This branch fixes this by replacing the `bool` field in `Wait` that's set on first poll with an "event count" stored in the remaining `WaitCell` state bits. Now, when a `Wait` is created, it loads the current event count, and calls to `wake()` and `close()` increment the event count. The `Wait` future then checks if the event count has gone up when it's polled, rather than just checking if it's ever been polled before. This allows the `Wait` future to determine if it is being polled because the `WaitCell` woke it up, or if it's being polled because some other future decided to poll it. This *also* has the side benefit of fixing racy scenarios where the `WaitCell` is woken between when the `Wait` future is created and when it's polled for the first time. Fixes #449
hawkw
added a commit
that referenced
this issue
Jul 23, 2023
This branch changes the `Wait` future for `maitake::sync::WaitCell` to handle spurious polls correctly. Currently, a `wait_cell::Wait` future assumes that if it's ever polled a second time, that means its waker was woken. However, there might be other reasons that a stack of futures containing a `Wait` is polled again, and the `Wait` future will incorrectly complete immediately in that case. This branch fixes this by replacing the `bool` field in `Wait` that's set on first poll with an additional bit stored in the `WaitCell`'s state field. This is set when the cell is actually woken, and only unset by the `Wait` future when it's polled. If the `WOKEN` bit was set, the `Wait` future completes, and if it was unset, the future re-registers itself. This way, the `Wait` future only completes if it was *woken by the waitcell*, rather than on *any* poll if the task was woken by something else. Fixes #449
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This test fails:
The second poll should return
Poll::Pending
, because theWaitCell
has not yet been woken. However:Shoutout to @jamesmunns for catching this oen!
The text was updated successfully, but these errors were encountered: