diff --git a/examples/service_struct_impl.rs b/examples/service_struct_impl.rs index 50fd3ab749..22cc2407dd 100644 --- a/examples/service_struct_impl.rs +++ b/examples/service_struct_impl.rs @@ -8,6 +8,7 @@ use tokio::net::TcpListener; use std::future::Future; use std::net::SocketAddr; use std::pin::Pin; +use std::sync::Mutex; type Counter = i32; @@ -23,7 +24,12 @@ async fn main() -> Result<(), Box> { tokio::task::spawn(async move { if let Err(err) = http1::Builder::new() - .serve_connection(stream, Svc { counter: 81818 }) + .serve_connection( + stream, + Svc { + counter: Mutex::new(81818), + }, + ) .await { println!("Failed to serve connection: {:?}", err); @@ -33,7 +39,7 @@ async fn main() -> Result<(), Box> { } struct Svc { - counter: Counter, + counter: Mutex, } impl Service> for Svc { @@ -41,7 +47,7 @@ impl Service> for Svc { type Error = hyper::Error; type Future = Pin> + Send>>; - fn call(&mut self, req: Request) -> Self::Future { + fn call(&self, req: Request) -> Self::Future { fn mk_response(s: String) -> Result>, hyper::Error> { Ok(Response::builder().body(Full::new(Bytes::from(s))).unwrap()) } @@ -58,7 +64,7 @@ impl Service> for Svc { }; if req.uri().path() != "/favicon.ico" { - self.counter += 1; + *self.counter.lock().expect("lock poisoned") += 1; } Box::pin(async { res }) diff --git a/src/service/service.rs b/src/service/service.rs index b5de9bec20..005406f982 100644 --- a/src/service/service.rs +++ b/src/service/service.rs @@ -28,5 +28,13 @@ pub trait Service { type Future: Future>; /// Process the request and return the response asynchronously. - fn call(&mut self, req: Request) -> Self::Future; + /// call takes a &self instead of a mut &self because: + /// - It prepares the way for async fn, + /// since then the future only borrows &self, and thus a Service can concurrently handle + /// multiple outstanding requests at once. + /// - It's clearer that Services can likely be cloned + /// - To share state across clones you generally need Arc> + /// that means you're not really using the &mut self and could do with a &self + /// To see the discussion on this see: https://github.com/hyperium/hyper/issues/3040 + fn call(&self, req: Request) -> Self::Future; } diff --git a/src/service/util.rs b/src/service/util.rs index 1d8587fe82..710ba53543 100644 --- a/src/service/util.rs +++ b/src/service/util.rs @@ -29,7 +29,7 @@ use crate::{Request, Response}; /// ``` pub fn service_fn(f: F) -> ServiceFn where - F: FnMut(Request) -> S, + F: Fn(Request) -> S, S: Future, { ServiceFn { @@ -46,7 +46,7 @@ pub struct ServiceFn { impl Service> for ServiceFn where - F: FnMut(Request) -> Ret, + F: Fn(Request) -> Ret, ReqBody: Body, Ret: Future, E>>, E: Into>, @@ -56,7 +56,7 @@ where type Error = E; type Future = Ret; - fn call(&mut self, req: Request) -> Self::Future { + fn call(&self, req: Request) -> Self::Future { (self.f)(req) } } diff --git a/tests/server.rs b/tests/server.rs index 632ce4839a..8561ab487c 100644 --- a/tests/server.rs +++ b/tests/server.rs @@ -2656,7 +2656,7 @@ impl Service> for TestService { type Error = BoxError; type Future = BoxFuture; - fn call(&mut self, mut req: Request) -> Self::Future { + fn call(&self, mut req: Request) -> Self::Future { let tx = self.tx.clone(); let replies = self.reply.clone(); @@ -2722,7 +2722,7 @@ impl Service> for HelloWorld { type Error = hyper::Error; type Future = future::Ready>; - fn call(&mut self, _req: Request) -> Self::Future { + fn call(&self, _req: Request) -> Self::Future { let response = Response::new(Full::new(HELLO.into())); future::ok(response) }