From 1d1d95d137598e4bf68f27e38d7265104c913148 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sat, 30 Dec 2023 22:15:34 +0100 Subject: [PATCH 1/3] testing clippy's disallowed-types --- axum/src/box_clone_service.rs | 89 +++++++++++++++++++++++++++++++++++ axum/src/lib.rs | 1 + clippy.toml | 3 ++ 3 files changed, 93 insertions(+) create mode 100644 axum/src/box_clone_service.rs create mode 100644 clippy.toml diff --git a/axum/src/box_clone_service.rs b/axum/src/box_clone_service.rs new file mode 100644 index 0000000000..85724e6ebf --- /dev/null +++ b/axum/src/box_clone_service.rs @@ -0,0 +1,89 @@ +use futures_util::future::BoxFuture; +use std::{ + fmt, + task::{Context, Poll}, +}; +use tower::ServiceExt; +use tower_layer::{layer_fn, LayerFn}; +use tower_service::Service; + +/// Like `tower::BoxCloneService` but `Sync` +pub(crate) struct BoxCloneService( + Box< + dyn CloneService>> + + Send + + Sync, + >, +); + +impl BoxCloneService { + pub(crate) fn new(inner: S) -> Self + where + S: Service + Clone + Send + Sync + 'static, + S::Future: Send + 'static, + { + let inner = inner.map_future(|f| Box::pin(f) as _); + BoxCloneService(Box::new(inner)) + } + + pub(crate) fn layer() -> LayerFn Self> + where + S: Service + Clone + Send + Sync + 'static, + S::Future: Send + 'static, + { + layer_fn(Self::new) + } +} + +impl Service for BoxCloneService { + type Response = U; + type Error = E; + type Future = BoxFuture<'static, Result>; + + #[inline] + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.0.poll_ready(cx) + } + + #[inline] + fn call(&mut self, request: T) -> Self::Future { + self.0.call(request) + } +} + +impl Clone for BoxCloneService { + fn clone(&self) -> Self { + Self(self.0.clone_box()) + } +} + +trait CloneService: Service { + fn clone_box( + &self, + ) -> Box< + dyn CloneService + + Send + + Sync, + >; +} + +impl CloneService for T +where + T: Service + Send + Sync + Clone + 'static, +{ + fn clone_box( + &self, + ) -> Box< + dyn CloneService + + Send + + Sync, + > { + Box::new(self.clone()) + } +} + +impl fmt::Debug for BoxCloneService { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.debug_struct("BoxCloneService").finish() + } +} diff --git a/axum/src/lib.rs b/axum/src/lib.rs index 601c14ae74..0057b9357d 100644 --- a/axum/src/lib.rs +++ b/axum/src/lib.rs @@ -420,6 +420,7 @@ #[macro_use] pub(crate) mod macros; +mod box_clone_service; mod boxed; mod extension; #[cfg(feature = "form")] diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000000..625309989d --- /dev/null +++ b/clippy.toml @@ -0,0 +1,3 @@ +disallowed-types = [ + { path = "tower::util::BoxCloneService", reason = "Use our internal BoxCloneService which is Sync" }, +] From cf51626e1a5a099cf6f72d65b942e2a807cf2aa8 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sat, 30 Dec 2023 22:28:04 +0100 Subject: [PATCH 2/3] Add `+ Sync` bounds everywhere --- axum-extra/src/handler/mod.rs | 2 +- axum-extra/src/handler/or.rs | 4 ++-- axum-extra/src/routing/mod.rs | 4 ++-- axum/src/box_clone_service.rs | 9 --------- axum/src/boxed.rs | 10 +++++----- axum/src/handler/mod.rs | 10 +++++----- axum/src/middleware/from_fn.rs | 6 ++++-- axum/src/routing/method_routing.rs | 21 +++++++++++---------- axum/src/routing/mod.rs | 20 ++++++++++---------- axum/src/routing/path_router.rs | 12 ++++++------ axum/src/routing/route.rs | 7 ++++--- 11 files changed, 50 insertions(+), 55 deletions(-) diff --git a/axum-extra/src/handler/mod.rs b/axum-extra/src/handler/mod.rs index 4017e149a6..5f4a1be122 100644 --- a/axum-extra/src/handler/mod.rs +++ b/axum-extra/src/handler/mod.rs @@ -168,7 +168,7 @@ pub struct IntoHandler { impl Handler for IntoHandler where - H: HandlerCallWithExtractors + Clone + Send + 'static, + H: HandlerCallWithExtractors + Clone + Send + Sync + 'static, T: FromRequest + Send + 'static, T::Rejection: Send, S: Send + Sync + 'static, diff --git a/axum-extra/src/handler/or.rs b/axum-extra/src/handler/or.rs index 0a468db70c..faa6851628 100644 --- a/axum-extra/src/handler/or.rs +++ b/axum-extra/src/handler/or.rs @@ -54,8 +54,8 @@ where impl Handler<(M, Lt, Rt), S> for Or where - L: HandlerCallWithExtractors + Clone + Send + 'static, - R: HandlerCallWithExtractors + Clone + Send + 'static, + L: HandlerCallWithExtractors + Clone + Send + Sync + 'static, + R: HandlerCallWithExtractors + Clone + Send + Sync + 'static, Lt: FromRequestParts + Send + 'static, Rt: FromRequest + Send + 'static, Lt::Rejection: Send, diff --git a/axum-extra/src/routing/mod.rs b/axum-extra/src/routing/mod.rs index ff6b7fde98..fc19268524 100644 --- a/axum-extra/src/routing/mod.rs +++ b/axum-extra/src/routing/mod.rs @@ -165,7 +165,7 @@ pub trait RouterExt: sealed::Sealed { /// This works like [`RouterExt::route_with_tsr`] but accepts any [`Service`]. fn route_service_with_tsr(self, path: &str, service: T) -> Self where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse, T::Future: Send + 'static, Self: Sized; @@ -268,7 +268,7 @@ where #[track_caller] fn route_service_with_tsr(mut self, path: &str, service: T) -> Self where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse, T::Future: Send + 'static, Self: Sized, diff --git a/axum/src/box_clone_service.rs b/axum/src/box_clone_service.rs index 85724e6ebf..25c0b205b8 100644 --- a/axum/src/box_clone_service.rs +++ b/axum/src/box_clone_service.rs @@ -4,7 +4,6 @@ use std::{ task::{Context, Poll}, }; use tower::ServiceExt; -use tower_layer::{layer_fn, LayerFn}; use tower_service::Service; /// Like `tower::BoxCloneService` but `Sync` @@ -25,14 +24,6 @@ impl BoxCloneService { let inner = inner.map_future(|f| Box::pin(f) as _); BoxCloneService(Box::new(inner)) } - - pub(crate) fn layer() -> LayerFn Self> - where - S: Service + Clone + Send + Sync + 'static, - S::Future: Send + 'static, - { - layer_fn(Self::new) - } } impl Service for BoxCloneService { diff --git a/axum/src/boxed.rs b/axum/src/boxed.rs index c734c2fb2f..2fb96f0962 100644 --- a/axum/src/boxed.rs +++ b/axum/src/boxed.rs @@ -33,7 +33,7 @@ impl BoxedIntoRoute { where S: 'static, E: 'static, - F: FnOnce(Route) -> Route + Clone + Send + 'static, + F: FnOnce(Route) -> Route + Clone + Send + Sync + 'static, E2: 'static, { BoxedIntoRoute(AxumMutex::new(Box::new(Map { @@ -59,7 +59,7 @@ impl fmt::Debug for BoxedIntoRoute { } } -pub(crate) trait ErasedIntoRoute: Send { +pub(crate) trait ErasedIntoRoute: Send + Sync { fn clone_box(&self) -> Box>; fn into_route(self: Box, state: S) -> Route; @@ -74,7 +74,7 @@ pub(crate) struct MakeErasedHandler { impl ErasedIntoRoute for MakeErasedHandler where - H: Clone + Send + 'static, + H: Clone + Send + Sync + 'static, S: 'static, { fn clone_box(&self) -> Box> { @@ -163,13 +163,13 @@ where } } -pub(crate) trait LayerFn: FnOnce(Route) -> Route + Send { +pub(crate) trait LayerFn: FnOnce(Route) -> Route + Send + Sync { fn clone_box(&self) -> Box>; } impl LayerFn for F where - F: FnOnce(Route) -> Route + Clone + Send + 'static, + F: FnOnce(Route) -> Route + Clone + Send + Sync + 'static, { fn clone_box(&self) -> Box> { Box::new(self.clone()) diff --git a/axum/src/handler/mod.rs b/axum/src/handler/mod.rs index 579fdbbad4..9a826414ef 100644 --- a/axum/src/handler/mod.rs +++ b/axum/src/handler/mod.rs @@ -131,7 +131,7 @@ pub use self::service::HandlerService; note = "Consider using `#[axum::debug_handler]` to improve the error message" ) )] -pub trait Handler: Clone + Send + Sized + 'static { +pub trait Handler: Clone + Send + Sync + Sized + 'static { /// The type of future calling this handler returns. type Future: Future + Send + 'static; @@ -192,7 +192,7 @@ pub trait Handler: Clone + Send + Sized + 'static { impl Handler<((),), S> for F where - F: FnOnce() -> Fut + Clone + Send + 'static, + F: FnOnce() -> Fut + Clone + Send + Sync + 'static, Fut: Future + Send, Res: IntoResponse, { @@ -210,7 +210,7 @@ macro_rules! impl_handler { #[allow(non_snake_case, unused_mut)] impl Handler<(M, $($ty,)* $last,), S> for F where - F: FnOnce($($ty,)* $last,) -> Fut + Clone + Send + 'static, + F: FnOnce($($ty,)* $last,) -> Fut + Clone + Send + Sync + 'static, Fut: Future + Send, S: Send + Sync + 'static, Res: IntoResponse, @@ -257,7 +257,7 @@ mod private { impl Handler for T where - T: IntoResponse + Clone + Send + 'static, + T: IntoResponse + Clone + Send + Sync + 'static, { type Future = std::future::Ready; @@ -302,7 +302,7 @@ where impl Handler for Layered where - L: Layer> + Clone + Send + 'static, + L: Layer> + Clone + Send + Sync + 'static, H: Handler, L::Service: Service + Clone + Send + 'static, >::Response: IntoResponse, diff --git a/axum/src/middleware/from_fn.rs b/axum/src/middleware/from_fn.rs index e4c44c74f5..d0318f9d9d 100644 --- a/axum/src/middleware/from_fn.rs +++ b/axum/src/middleware/from_fn.rs @@ -1,3 +1,4 @@ +use crate::box_clone_service::BoxCloneService; use crate::response::{IntoResponse, Response}; use axum_core::extract::{FromRequest, FromRequestParts, Request}; use futures_util::future::BoxFuture; @@ -10,7 +11,7 @@ use std::{ pin::Pin, task::{Context, Poll}, }; -use tower::{util::BoxCloneService, ServiceBuilder}; +use tower::ServiceBuilder; use tower_layer::Layer; use tower_service::Service; @@ -259,6 +260,7 @@ macro_rules! impl_service { I: Service + Clone + Send + + Sync + 'static, I::Response: IntoResponse, I::Future: Send + 'static, @@ -297,7 +299,7 @@ macro_rules! impl_service { }; let inner = ServiceBuilder::new() - .boxed_clone() + .layer_fn(BoxCloneService::new) .map_response(IntoResponse::into_response) .service(ready_inner); let next = Next { inner }; diff --git a/axum/src/routing/method_routing.rs b/axum/src/routing/method_routing.rs index 355dda9904..56ef3bec1f 100644 --- a/axum/src/routing/method_routing.rs +++ b/axum/src/routing/method_routing.rs @@ -78,7 +78,7 @@ macro_rules! top_level_service_fn { $(#[$m])+ pub fn $name(svc: T) -> MethodRouter where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, S: Clone, @@ -210,6 +210,7 @@ macro_rules! chained_service_fn { T: Service + Clone + Send + + Sync + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, @@ -312,7 +313,7 @@ top_level_service_fn!(trace_service, TRACE); /// ``` pub fn on_service(filter: MethodFilter, svc: T) -> MethodRouter where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, S: Clone, @@ -371,7 +372,7 @@ where /// ``` pub fn any_service(svc: T) -> MethodRouter where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, S: Clone, @@ -736,7 +737,7 @@ where #[track_caller] pub fn on_service(self, filter: MethodFilter, svc: T) -> Self where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, { @@ -868,7 +869,7 @@ where #[doc = include_str!("../docs/method_routing/fallback.md")] pub fn fallback_service(mut self, svc: T) -> Self where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, { @@ -879,8 +880,8 @@ where #[doc = include_str!("../docs/method_routing/layer.md")] pub fn layer(self, layer: L) -> MethodRouter where - L: Layer> + Clone + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L: Layer> + Clone + Send + Sync + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Response: IntoResponse + 'static, >::Error: Into + 'static, >::Future: Send + 'static, @@ -908,8 +909,8 @@ where #[track_caller] pub fn route_layer(mut self, layer: L) -> MethodRouter where - L: Layer> + Clone + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L: Layer> + Clone + Send + Sync + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Response: IntoResponse + 'static, >::Future: Send + 'static, E: 'static, @@ -1151,7 +1152,7 @@ where where S: 'static, E: 'static, - F: FnOnce(Route) -> Route + Clone + Send + 'static, + F: FnOnce(Route) -> Route + Clone + Send + Sync + 'static, E2: 'static, { match self { diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 6564df7d62..8fb8a7fb5f 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -165,7 +165,7 @@ where #[doc = include_str!("../docs/routing/route_service.md")] pub fn route_service(self, path: &str, service: T) -> Self where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse, T::Future: Send + 'static, { @@ -210,7 +210,7 @@ where #[track_caller] pub fn nest_service(self, path: &str, service: T) -> Self where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse, T::Future: Send + 'static, { @@ -274,8 +274,8 @@ where #[doc = include_str!("../docs/routing/layer.md")] pub fn layer(self, layer: L) -> Router where - L: Layer + Clone + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L: Layer + Clone + Send + Sync + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Response: IntoResponse + 'static, >::Error: Into + 'static, >::Future: Send + 'static, @@ -292,8 +292,8 @@ where #[track_caller] pub fn route_layer(self, layer: L) -> Self where - L: Layer + Clone + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L: Layer + Clone + Send + Sync + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Response: IntoResponse + 'static, >::Error: Into + 'static, >::Future: Send + 'static, @@ -325,7 +325,7 @@ where /// See [`Router::fallback`] for more details. pub fn fallback_service(self, service: T) -> Self where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse, T::Future: Send + 'static, { @@ -632,7 +632,7 @@ where where S: 'static, E: 'static, - F: FnOnce(Route) -> Route + Clone + Send + 'static, + F: FnOnce(Route) -> Route + Clone + Send + Sync + 'static, E2: 'static, { match self { @@ -695,8 +695,8 @@ where { fn layer(self, layer: L) -> Endpoint where - L: Layer + Clone + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L: Layer + Clone + Send + Sync + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Response: IntoResponse + 'static, >::Error: Into + 'static, >::Future: Send + 'static, diff --git a/axum/src/routing/path_router.rs b/axum/src/routing/path_router.rs index e9353dc748..5b8a3b7b86 100644 --- a/axum/src/routing/path_router.rs +++ b/axum/src/routing/path_router.rs @@ -85,7 +85,7 @@ where service: T, ) -> Result<(), Cow<'static, str>> where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse, T::Future: Send + 'static, { @@ -204,7 +204,7 @@ where svc: T, ) -> Result<(), Cow<'static, str>> where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse, T::Future: Send + 'static, { @@ -239,8 +239,8 @@ where pub(super) fn layer(self, layer: L) -> PathRouter where - L: Layer + Clone + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L: Layer + Clone + Send + Sync + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Response: IntoResponse + 'static, >::Error: Into + 'static, >::Future: Send + 'static, @@ -264,8 +264,8 @@ where #[track_caller] pub(super) fn route_layer(self, layer: L) -> Self where - L: Layer + Clone + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L: Layer + Clone + Send + Sync + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Response: IntoResponse + 'static, >::Error: Into + 'static, >::Future: Send + 'static, diff --git a/axum/src/routing/route.rs b/axum/src/routing/route.rs index 2bde8c8c5f..149a703956 100644 --- a/axum/src/routing/route.rs +++ b/axum/src/routing/route.rs @@ -1,5 +1,6 @@ use crate::{ body::{Body, HttpBody}, + box_clone_service::BoxCloneService, response::Response, util::AxumMutex, }; @@ -18,7 +19,7 @@ use std::{ task::{Context, Poll}, }; use tower::{ - util::{BoxCloneService, MapErrLayer, MapRequestLayer, MapResponseLayer, Oneshot}, + util::{MapErrLayer, MapRequestLayer, MapResponseLayer, Oneshot}, ServiceExt, }; use tower_layer::Layer; @@ -33,7 +34,7 @@ pub struct Route(AxumMutex impl Route { pub(crate) fn new(svc: T) -> Self where - T: Service + Clone + Send + 'static, + T: Service + Clone + Send + Sync + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, { @@ -52,7 +53,7 @@ impl Route { pub(crate) fn layer(self, layer: L) -> Route where L: Layer> + Clone + Send + 'static, - L::Service: Service + Clone + Send + 'static, + L::Service: Service + Clone + Send + Sync + 'static, >::Response: IntoResponse + 'static, >::Error: Into + 'static, >::Future: Send + 'static, From 5b316ebc64fed4df4d9153b57d7fe2e58cbadb4b Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sat, 30 Dec 2023 22:50:56 +0100 Subject: [PATCH 3/3] changelog --- axum/CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index 80ce95c379..ea92b346a4 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -7,9 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 # Unreleased +- **breaking:** Require `Sync` for all handlers and services added to `Router` + and `MethodRouter` ([#2473]) - **fixed:** Fixed layers being cloned when calling `axum::serve` directly with a `Router` or `MethodRouter` ([#2586]) +[#2473]: https://github.com/tokio-rs/axum/pull/2473 [#2586]: https://github.com/tokio-rs/axum/pull/2586 # 0.7.4 (13. January, 2024) @@ -21,7 +24,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2201]: https://github.com/tokio-rs/axum/pull/2201 [#2483]: https://github.com/tokio-rs/axum/pull/2483 -[#2201]: https://github.com/tokio-rs/axum/pull/2201 [#2484]: https://github.com/tokio-rs/axum/pull/2484 # 0.7.3 (29. December, 2023)