From 52fd139a862bfed2e910352dc93eeb142c2f83c9 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sat, 28 Sep 2024 23:28:44 +0200 Subject: [PATCH] Require `Sync` for services (#2473) Co-authored-by: Jonas Platte --- axum-extra/src/handler/mod.rs | 2 +- axum-extra/src/handler/or.rs | 4 +- axum-extra/src/routing/mod.rs | 4 +- axum/CHANGELOG.md | 4 +- axum/src/box_clone_service.rs | 80 ++++++++++++++++++++++++++++++ axum/src/boxed.rs | 10 ++-- axum/src/handler/mod.rs | 10 ++-- axum/src/lib.rs | 1 + 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 +-- clippy.toml | 3 ++ 14 files changed, 137 insertions(+), 47 deletions(-) create mode 100644 axum/src/box_clone_service.rs create mode 100644 clippy.toml diff --git a/axum-extra/src/handler/mod.rs b/axum-extra/src/handler/mod.rs index 915852a1..c3bf7024 100644 --- a/axum-extra/src/handler/mod.rs +++ b/axum-extra/src/handler/mod.rs @@ -165,7 +165,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 0a468db7..faa68516 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 a294c547..bf99bbcf 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/CHANGELOG.md b/axum/CHANGELOG.md index b897e757..47b69e2c 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]) - **breaking:** The tuple and tuple_struct `Path` extractor deserializers now check that the number of parameters matches the tuple length exactly ([#2931]) - **change:** Update minimum rust version to 1.75 ([#2943]) +[#2473]: https://github.com/tokio-rs/axum/pull/2473 [#2931]: https://github.com/tokio-rs/axum/pull/2931 [#2943]: https://github.com/tokio-rs/axum/pull/2943 @@ -55,7 +58,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) diff --git a/axum/src/box_clone_service.rs b/axum/src/box_clone_service.rs new file mode 100644 index 00000000..25c0b205 --- /dev/null +++ b/axum/src/box_clone_service.rs @@ -0,0 +1,80 @@ +use futures_util::future::BoxFuture; +use std::{ + fmt, + task::{Context, Poll}, +}; +use tower::ServiceExt; +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)) + } +} + +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/boxed.rs b/axum/src/boxed.rs index 32808f51..6347de58 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; @@ -75,7 +75,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> { @@ -165,13 +165,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 04c66c4a..8485390b 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/lib.rs b/axum/src/lib.rs index 64d4a6a2..27b87e90 100644 --- a/axum/src/lib.rs +++ b/axum/src/lib.rs @@ -463,6 +463,7 @@ #[macro_use] pub(crate) mod macros; +mod box_clone_service; mod boxed; mod extension; #[cfg(feature = "form")] diff --git a/axum/src/middleware/from_fn.rs b/axum/src/middleware/from_fn.rs index f0f47611..e34fd058 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; @@ -260,6 +261,7 @@ macro_rules! impl_service { I: Service + Clone + Send + + Sync + 'static, I::Response: IntoResponse, I::Future: Send + 'static, @@ -298,7 +300,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 1eb6075b..8c50eccf 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 dc6ca815..4bd8b28b 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, { @@ -211,7 +211,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, { @@ -275,8 +275,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, @@ -293,8 +293,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, @@ -331,7 +331,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, { @@ -638,7 +638,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 { @@ -701,8 +701,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 e132f1dc..6a7d3cee 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, { @@ -201,7 +201,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, { @@ -236,8 +236,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, @@ -261,8 +261,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 242e25f3..71349ba6 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, diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000..62530998 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,3 @@ +disallowed-types = [ + { path = "tower::util::BoxCloneService", reason = "Use our internal BoxCloneService which is Sync" }, +]