Update tower-util and tower to std::future#330
Merged
Conversation
This was referenced Sep 10, 2019
hawkw
added a commit
that referenced
this pull request
Apr 23, 2020
## Motivation Commit #330 introduced a regression when porting `tower-util::Oneshot` from `futures` 0.1 to `std::future`. The *intended* behavior is that a oneshot future should repeatedly call `poll_ready` on the oneshotted service until it is ready, and then call the service and drive the returned future. However, #330 inadvertently changed the oneshot future to poll the service _once_, call it if it is ready, and then drop it, regardless of its readiness. In the #330 version of oneshot, an `Option` is used to store the request while waiting for the service to become ready, so that it can be `take`n and moved into the service's `call`. However, the `Option` contains both the request _and_ the service itself, and is taken the first time the service is polled. `futures::ready!` is then used when polling the service, so the method returns immediate if it is not ready. This means that the service itself (and the request), which were taken out of the `Option`, will be dropped, and if the oneshot future is polled again, it will panic. ## Solution This commit changes the `Oneshot` future so that only the request lives in the `Option`, and it is only taken when the service is called, rather than every time it is polled. This fixes the bug. I've also added a test for this which fails against master, but passes after this change. Signed-off-by: Eliza Weisman <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This bumps tower-util and tower to 0.3.0-alpha.1.
One noteworthy change here is that
CallAllnow also has atake_servicemethod that can be used through aPin. Not sure whether it is necessary, but it was handy for tests.