`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>
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>
In many cases, new releases of a dependency can break compatibility with
Tower's minimum supported Rust version (MSRV). It shouldn't be necessary
for Tower to bump its MSRV when a dependency does, as users on older
Rust versions should be able to depend on older versions of that crate.
Instead, we should probably just run our MSRV checks with minimal
dependency versions.
This branch changes Tower's CI jobs to do that. It was also necessary to
make some changes to the `Cargo.toml` to actually fix the build with
minimal dependency versions.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
`tower` builds are now failing on CI because Tokio v1.17.0 bumped MSRV
to 1.49.0. This branch updates `tower`'s MSRV to 1.49.0 to track Tokio's
MSRV. I also added nicer documentation of the MSRV based on Tokio's, and
added the `rust-version` Cargo metadata to the `tower` crate's
`Cargo.toml`.
Note that `tower-service` and `tower-layer` can technically continue to
support much earlier Rust versions than `tower` can, since they don't
depend on external crates and are very small. We could consider testing
separate, older MSRVs on CI for those crates individually. I didn't do
that in this PR, though, because I wasn't sure if this was worth the
effort and I just wanted to get CI passing again.
This PR updates `tokio-util` to v0.7.
It also updates the minimum `tokio` dependency to v1.6.0.
This is because `tokio-util` requires at least `tokio` v1.6.0 for
`mpsc::Sender::reserve_owned`, but it only specifies a minimum version
of v1.0.0. This is incorrect and should be considered an upstream bug,
but updating our tokio dep fixes this, so that should at least unbreak
`tower`'s build for now.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This fixes a bunch of minor clippy lints. None of them were particularly
major, but I was getting tired of the warnings showing up in vscode.
The one lint that had to be ignored rather than fixed is the
`clippy::bool_assert_comparison` lint, which triggers on the
`tower_test::assert_request_eq!` macro. The lint triggers when writing
code like `assert_eq!(whatever, true)` rather than simply
`assert!(whatever)`. In this case, this occurs because the macro makes
an assertion about a request value, and in _some_ tests, the request
type is `bool`. We can't change this to use `assert!`, because in most
cases, when the request is not `bool`, we actually do need `assert_eq!`,
so I ignored that warning.
We have reports of runtime panics (linkerd/linkerd2#7748) that sound a
lot like rust-lang/rust#86470. We don't have any evidence that these
panics originate in tower, but we have some potentialy flawed `Instant`
arithmetic that could panic in this way.
Even though this is almost definitely a bug in Rust, it seems most
prudent to actively avoid the uses of `Instant` that are prone to this
bug.
This change replaces uses of `Instant::elapsed` and `Instant::sub` with
calls to `Instant::saturating_duration_since` to prevent this class of
panic. These fixes should ultimately be made in the standard library,
but this change lets us avoid this problem while we wait for those
fixes.
See also hyperium/hyper#2746
`tower` currently has required dependencies that may not be used
unless certain features are enabled.
This change updates `tower` to make these dependencies optional.
Furthermore, this change removes use of `html_root_url`, which is no
longer recommended (https://github.com/rust-lang/api-guidelines/pull/230),
and updates the documented release instructions.
The tower crates do not include any `unsafe` code, but tools like
[`cargo-geiger`][cg] can't necessarily detect that. This change adds a
stronger assertion at the top of each crate with a
`#![forbid(unsafe_code)]` directive to assert that no unsafe code is
used in the crate. This also serves as a more obvious obstacle to
introducing unsafe code in future changes.
[cg]: https://github.com/rust-secure-code/cargo-geiger
* builder,util: add convenience methods for boxing services
This adds a couple of new methods to `ServiceBuilder` and `ServiceExt`:
- `ServiceBuilder::boxed`
- `ServiceExt::boxed`
- `ServiceBuilder::clone_boxed`
- `ServiceExt::clone_boxed`
They apply `BoxService::layer` and `CloneBoxService::layer`
respectively.
* fix doc links
* add missing `cfg`s
* Update tower/CHANGELOG.md
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
* Apply suggestions from code review
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
* not sure why rustdoc cannot infer these
* line breaks
* trailing whitespace
* make docs a bit more consistent
* fix doc links
* update tokio
* don't pull in old version of tower
* Don't run `cargo deny check bans` as it hangs
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
* util: add `CloneService`
This upstreams a little utility I'm using a bunch in axum. Its often
useful to erase the type of a service while still being able to clone
it.
`BoxService` isn't `Clone` previously you had to combine it with
`Buffer` but doing that a lot (which we did in axum) had measurable
impact on performance.
* Address review feedback
* remove needless trait bounds
The balancer types derive `Debug` and `Clone` implementations, but
this unnecessarily requires that its type parameters implement these
traits.
This change provides manual implementations for `Clone` and `Debug`
to avoid this unintentional restriction.
Closes#606
* tower: prepare to release 0.4.10
- Fix accidental breaking change when using the
`rustdoc::broken_intra_doc_links` lint ([#605])
- Clarity that tower's minimum supported rust version is 1.46 ([#605])
[#605]: https://github.com/tower-rs/tower/pull/605
* Update tower/CHANGELOG.md
Co-authored-by: Oliver Gould <ver@buoyant.io>
Co-authored-by: Oliver Gould <ver@buoyant.io>
* Actually check MSCV on CI
* check broken_intra_doc_links on CI
* clean up CI
* fix other crates
* bump MSRV to 1.42 because of tracing-core
* attempt to fix http not working on 1.42
* use `--workspace` instead of `--all`
`--all` is deprecated
* make tower-service build on 1.42
* force http version 0.2.4
* actually msrv is 1.46 because of tokio
* fix running `cargo fmt`
* clean up
* also run tests on 1.46
* ignore rustsec in time
* REMOVE ME updates peak_wema test to pass
* adds pin_project_lite dependency
* uses pin_project_lite for load::Constant
* uses pin_project_lite for load::PencingRequestsDiscover
* uses pin_project_lite for load::PeakEwma
* uses pin_project_lite for load::Completion
* uses pin_project_lite for tests::support::IntoStream
Turns IntoStream into a regular struct because pin_project_lite does not and will support tuple structs.
https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408
* refactors opaque_future into a regular struct
This enables migration to pin_project_lite, which does not and will not support tuple structs
https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408
* migrates opaque_future to use pin_project_lite
* removes tuple variant from load_shed::ResponseState enum
* migrates load_shed::future to pin_project_lite
* removes tuple variant from filter::future::State
* migrates filter::future to pin_project_lite
Note: the doc comment on AsyncResponseFuture::service was also reduced to a regular comment.
This is a known limitation of pin_project_lite that the they have labeled as "help wanted".
https://github.com/taiki-e/pin-project-lite/issues/3#issuecomment-745194112
* migrates retry::Retry to pin_project_lite
* refactors retry::future::State to enable pin_project_lite
pin_project_lite has the current limitation of nto supporting doc comments
https://github.com/taiki-e/pin-project-lite/issues/3#issuecomment-745194112
pin_project_lite does not and will not support tuple variants
https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408
* migrates retry::future to pin_project_lite
* migrates spawn_ready::make to pin_project_lite
* refactors buffer::future::ResponseState to allow pin_project_lite
* migrates buffer::future to pin_project_lite
* refactors util::AndThenFuture to allow pin_project_lite
* migrates util::AndThenFuture to pin_project_lite
* migrates hedge::Future to pin_project_lite
* migrates hedge::select::ResponseFuture to pin_project_lite
* refactors hedge::delay enum for pin_project_lite
* refactors reconnect::future enum for pin_project_lite
* refactors oneshot::State enum for pin_project_lite
* migrates util::oneshot to pin_project_lite
* migrates reconnect::future to pin_project_lite
* migrates hedge::delay to pin_project_lite
* migrates hedge::latency to pin_project_lite
* migrates discover::list to pin_project_lite
* migrates timeout::future to pin_project_lite
* migrates balance::pool to pin_project_lite
* migrates balance::p2c::make to pin_project_lite
* migrates balance::p2c::service to pin_project_lite
* migrates call_all::ordered to pin_project_lite
* migrates call_all::common to pin_project_lite
* migrates call_all::unordered to pin_project_lite
* migrates util::optional::future to pin_project_lite
* migrates limit::concurrency::future to pin_project_lite
* migrates tower-balance example to pin_project_lite
* applies cargo fmt
* migrates tower-test to pin_project_lite
* fixes cargo hack check
peak_wma and pending_requests will now properly compile without the "discover" feature enabled.
* fixes lint rename warning on nightly
broken_intra_doc_links has been renamed to rustdoc::broken_intra_doc_links
* migrates buffer::Worker to pin_project_lite
pin_project_lite does support PinnedDrop
https://github.com/taiki-e/pin-project-lite/pull/25/files
However, it does not support generic trait bounds on the PinnedDrop impl.
To workaround this, I removed the T::Error bound from the Worker struct definition,
and moved `close_semaphore` to a a new impl without that trait bound.
* fixes abort_on_drop test
This test was also failing on master.
* applies cargo fmt
* limit: global concurrency limit layer from a owned semaphore
* new_owned -> new_shared + docs improvements
Co-authored-by: David Pedersen <david.pdrsn@gmail.com>
* keep exposing Semaphore, but rename the API a bit and make it simpler to use
* missed a spot
* minor docs fixes
* update changelog
Co-authored-by: David Pedersen <david.pdrsn@gmail.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
This adds the first Tower guide called "Inventing the `Service` trait". It attempts to motivate all the parts to `Service` by walking the user through how they could have invented `Service` themselves, from scratch. It goes into quite a bit of detail but hopefully it paints a somewhat complete picture in the end.
The next guide I want to write is about how to implement a proper `Timeout` middleware using `Layer`, pin-project, and all the bells and whistles.
Ref: https://github.com/tower-rs/tower/issues/33
# 0.4.7 (April 27, 2021)
### Added
- **builder**: Add `ServiceBuilder::check_service` to check the request,
response, and error types of the output service. ([#576])
- **builder**: Add `ServiceBuilder::check_service_clone` to check the
output service can be cloned. ([#576])
### Fixed
- **spawn_ready**: Abort spawned background tasks when the `SpawnReady`
service is dropped, fixing a potential task/resource leak (#[581])
- Fixed broken documentation links ([#578])
[#576]: https://github.com/tower-rs/tower/pull/576
[#578]: https://github.com/tower-rs/tower/pull/578
[#581]: https://github.com/tower-rs/tower/pull/581
The `SpawnReady` service spawns a background task whenever the inner
service is not ready; but when the `SpawnReady` service is dropped, this
task continues to run until it becomes ready (at which point the ready
service will be dropped). This can cause resource leaks when the inner
service never becomes ready.
This change adds a `Drop` implementation for the `SpawnReady` service
that aborts the background task when one is present.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
I've run into a use case where I need to return a `Layer` with a complex
type from a function. I previously used `impl Layer<S, Service = impl
Service<...>>` but that impacted compile times quite significantly.
Boxing the `Layer` fixed it. Thought it made sense to upstream.
The `Send + Sync + 'static` bounds on the inner `dyn Layer` were
necessary to get it working with hyper.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
* builder: Add `ServiceBuilder::service_fn`
A small convenience for doing `.service(service_fn(handler_function))`.
* Docs tweaks
* Apply suggestions from code review
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
* Fix
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
This PR renames:
- `ServiceExt::ready_and` to `ServiceExt::ready`
- the `ReadyAnd` future to `Ready`
- the associated documentation to refer to `ServiceExt::ready`
and `ReadyAnd`.
This PR deprecates:
- the `ServiceExt::ready_and` method
- the `ReadyAnd` future
These can be removed in Tower 0.5.
My recollection of the original conversation surrounding the
introduction of the `ServiceExt::ready_and` combinator in
https://github.com/tower-rs/tower/pull/427 was that it was meant to be a
temporary workaround for the unchainable `ServiceExt::ready` combinator
until the next breaking release of the Tower crate. The unchainable
`ServiceExt::ready` combinator was removed, but `ServiceExt::ready_and`
was not renamed. I believe, but am not 100% sure, that this was an
oversight.
Eliza Weisman