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

backport non-breaking changes from master to v0.4.x #671

Merged
merged 7 commits into from
Jun 17, 2022
Merged

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jun 17, 2022

This branch backports all of the non-breaking changes made on master
since master was bumped to tower 0.5:

mgrachev and others added 7 commits June 17, 2022 12:08
Currently `call_all` will hang in a busy loop if called when the input
stream is pending.
…y() (#662)

Signed-off-by: Matt Klein <mklein@lyft.com>
This allows for mocking. This also matches what is done for
the timeout Elapsed error.

Signed-off-by: Matt Klein <mklein@lyft.com>
This gets rid of the `Unpin` impl with the weird comment on it.

Alternatively, we could just put a `S: Unpin` bound on `Pending`, but
this changes the public API to require that the service type is `Unpin`.
In practice, it will be, but we could also just avoid the trait bound.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
`tokio::task` enforces a cooperative scheduling regime that can cause
`oneshot::Receiver::poll` to return pending after the sender has sent an
update. `ReadyCache` uses a oneshot to notify pending services that they
should not become ready. When a cancelation is not observed, the ready
cache return service instances that should have been canceled, which
breaks assumptions and causes an invalid state.

This branch replaces the use of `tokio::sync::oneshot` for canceling 
pending futures with a custom cancelation handle using an `AtomicBool`
and `futures::task::AtomicWaker`. This ensures that canceled `Pending`
services are always woken even when the task's budget is exceeded.
Additionally, cancelation status is now always known to the `Pending`
future, by checking the `AtomicBool` immediately on polls, even in cases
where the canceled `Pending` future was woken by the inner `Service`
becoming ready, rather than by the cancelation.

Fixes #415

Signed-off-by: Oliver Gould <ver@buoyant.io>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw requested review from olix0r and LucioFranco June 17, 2022 19:13
@hawkw hawkw self-assigned this Jun 17, 2022
@hawkw hawkw enabled auto-merge (rebase) June 17, 2022 19:13
@olix0r
Copy link
Collaborator

olix0r commented Jun 17, 2022

I think you'll need #670 as well?

@hawkw hawkw merged commit 051b094 into v0.4.x Jun 17, 2022
@hawkw hawkw deleted the eliza/backport-668 branch June 17, 2022 19:18
@hawkw
Copy link
Member Author

hawkw commented Jun 17, 2022

I think you'll need #670 as well?

Already did that when I opened the v0.4.x branch :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants