From a2d4ff157ea981a09d56e4284fd484e88a5c498d Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Tue, 28 Jan 2020 08:09:46 +0900 Subject: [PATCH 1/3] Update `call_service` documentation (#1302) Co-authored-by: Christian Battaglia --- src/test.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test.rs b/src/test.rs index bb0d05cf..95698053 100644 --- a/src/test.rs +++ b/src/test.rs @@ -95,11 +95,10 @@ where /// Calls service and waits for response future completion. /// /// ```rust -/// use actix_web::{test, App, HttpResponse, http::StatusCode}; -/// use actix_service::Service; +/// use actix_web::{test, web, App, HttpResponse, http::StatusCode}; /// -/// #[test] -/// fn test_response() { +/// #[actix_rt::test] +/// async fn test_response() { /// let mut app = test::init_service( /// App::new() /// .service(web::resource("/test").to(|| async { From d137a8635b455a8b36c1df7a4ec9d698cbf27468 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 27 Jan 2020 20:45:26 -0500 Subject: [PATCH 2/3] Replace `Pin::new_unchecked` with #[pin_project] in `tuple_from_req!` (#1293) Using some module trickery, we can generate a tuple struct for each invocation of the macro. This allows us to use `pin_project` to project through to the tuple fields, removing the need to use `Pin::new_unchecked` Co-authored-by: Yuki Okushi --- src/extract.rs | 108 ++++++++++++++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 41 deletions(-) diff --git a/src/extract.rs b/src/extract.rs index c189bbf9..5289bd7d 100644 --- a/src/extract.rs +++ b/src/extract.rs @@ -193,57 +193,83 @@ impl FromRequest for () { macro_rules! tuple_from_req ({$fut_type:ident, $(($n:tt, $T:ident)),+} => { - /// FromRequest implementation for tuple - #[doc(hidden)] - #[allow(unused_parens)] - impl<$($T: FromRequest + 'static),+> FromRequest for ($($T,)+) - { - type Error = Error; - type Future = $fut_type<$($T),+>; - type Config = ($($T::Config),+); + // This module is a trick to get around the inability of + // `macro_rules!` macros to make new idents. We want to make + // a new `FutWrapper` struct for each distinct invocation of + // this macro. Ideally, we would name it something like + // `FutWrapper_$fut_type`, but this can't be done in a macro_rules + // macro. + // + // Instead, we put everything in a module named `$fut_type`, thus allowing + // us to use the name `FutWrapper` without worrying about conflicts. + // This macro only exists to generate trait impls for tuples - these + // are inherently global, so users don't have to care about this + // weird trick. + #[allow(non_snake_case)] + mod $fut_type { - fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { - $fut_type { - items: <($(Option<$T>,)+)>::default(), - futs: ($($T::from_request(req, payload),)+), + // Bring everything into scope, so we don't need + // redundant imports + use super::*; + + /// A helper struct to allow us to pin-project through + /// to individual fields + #[pin_project::pin_project] + struct FutWrapper<$($T: FromRequest),+>($(#[pin] $T::Future),+); + + /// FromRequest implementation for tuple + #[doc(hidden)] + #[allow(unused_parens)] + impl<$($T: FromRequest + 'static),+> FromRequest for ($($T,)+) + { + type Error = Error; + type Future = $fut_type<$($T),+>; + type Config = ($($T::Config),+); + + fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { + $fut_type { + items: <($(Option<$T>,)+)>::default(), + futs: FutWrapper($($T::from_request(req, payload),)+), + } } } - } - #[doc(hidden)] - #[pin_project::pin_project] - pub struct $fut_type<$($T: FromRequest),+> { - items: ($(Option<$T>,)+), - futs: ($($T::Future,)+), - } + #[doc(hidden)] + #[pin_project::pin_project] + pub struct $fut_type<$($T: FromRequest),+> { + items: ($(Option<$T>,)+), + #[pin] + futs: FutWrapper<$($T,)+>, + } - impl<$($T: FromRequest),+> Future for $fut_type<$($T),+> - { - type Output = Result<($($T,)+), Error>; + impl<$($T: FromRequest),+> Future for $fut_type<$($T),+> + { + type Output = Result<($($T,)+), Error>; - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let this = self.project(); + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let mut this = self.project(); - let mut ready = true; - $( - if this.items.$n.is_none() { - match unsafe { Pin::new_unchecked(&mut this.futs.$n) }.poll(cx) { - Poll::Ready(Ok(item)) => { - this.items.$n = Some(item); + let mut ready = true; + $( + if this.items.$n.is_none() { + match this.futs.as_mut().project().$n.poll(cx) { + Poll::Ready(Ok(item)) => { + this.items.$n = Some(item); + } + Poll::Pending => ready = false, + Poll::Ready(Err(e)) => return Poll::Ready(Err(e.into())), } - Poll::Pending => ready = false, - Poll::Ready(Err(e)) => return Poll::Ready(Err(e.into())), } - } - )+ + )+ - if ready { - Poll::Ready(Ok( - ($(this.items.$n.take().unwrap(),)+) - )) - } else { - Poll::Pending - } + if ready { + Poll::Ready(Ok( + ($(this.items.$n.take().unwrap(),)+) + )) + } else { + Poll::Pending + } + } } } }); From 74dcc7366d8ffa0316d4fccf773dff0ff2bb94c8 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 27 Jan 2020 22:35:51 -0500 Subject: [PATCH 3/3] Remove several uses of `Pin::new_unchecked` (#1294) Most of the relevant struct already had a `#[pin_project]` attribute, but it wasn't being used. The remaining uses of `Pin::new_unchecked` all involve going from a `&mut T` to a `Pin<&mut T>`, without directly observing a `Pin<&mut T>` first. As such, they cannot be replaced by `pin_project` Co-authored-by: Yuki Okushi --- actix-http/src/client/pool.rs | 19 +++++++++++-------- actix-http/src/h2/dispatcher.rs | 19 +++++++++++-------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/actix-http/src/client/pool.rs b/actix-http/src/client/pool.rs index acf76559..8c94423a 100644 --- a/actix-http/src/client/pool.rs +++ b/actix-http/src/client/pool.rs @@ -488,10 +488,12 @@ where } } +#[pin_project::pin_project(PinnedDrop)] struct OpenWaitingConnection where Io: AsyncRead + AsyncWrite + Unpin + 'static, { + #[pin] fut: F, key: Key, h2: Option< @@ -525,12 +527,13 @@ where } } -impl Drop for OpenWaitingConnection +#[pin_project::pinned_drop] +impl PinnedDrop for OpenWaitingConnection where Io: AsyncRead + AsyncWrite + Unpin + 'static, { - fn drop(&mut self) { - if let Some(inner) = self.inner.take() { + fn drop(self: Pin<&mut Self>) { + if let Some(inner) = self.project().inner.take() { let mut inner = inner.as_ref().borrow_mut(); inner.release(); inner.check_availibility(); @@ -545,8 +548,8 @@ where { type Output = (); - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let this = unsafe { self.get_unchecked_mut() }; + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let this = self.as_mut().project(); if let Some(ref mut h2) = this.h2 { return match Pin::new(h2).poll(cx) { @@ -571,7 +574,7 @@ where }; } - match unsafe { Pin::new_unchecked(&mut this.fut) }.poll(cx) { + match this.fut.poll(cx) { Poll::Ready(Err(err)) => { let _ = this.inner.take(); if let Some(rx) = this.rx.take() { @@ -589,8 +592,8 @@ where ))); Poll::Ready(()) } else { - this.h2 = Some(handshake(io).boxed_local()); - unsafe { Pin::new_unchecked(this) }.poll(cx) + *this.h2 = Some(handshake(io).boxed_local()); + self.poll(cx) } } Poll::Pending => Poll::Pending, diff --git a/actix-http/src/h2/dispatcher.rs b/actix-http/src/h2/dispatcher.rs index a4ec15fa..8b17e947 100644 --- a/actix-http/src/h2/dispatcher.rs +++ b/actix-http/src/h2/dispatcher.rs @@ -158,14 +158,16 @@ where #[pin_project::pin_project] struct ServiceResponse { + #[pin] state: ServiceResponseState, config: ServiceConfig, buffer: Option, _t: PhantomData<(I, E)>, } +#[pin_project::pin_project] enum ServiceResponseState { - ServiceCall(F, Option>), + ServiceCall(#[pin] F, Option>), SendPayload(SendStream, ResponseBody), } @@ -247,12 +249,14 @@ where { type Output = (); + #[pin_project::project] fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let mut this = self.as_mut().project(); - match this.state { - ServiceResponseState::ServiceCall(ref mut call, ref mut send) => { - match unsafe { Pin::new_unchecked(call) }.poll(cx) { + #[project] + match this.state.project() { + ServiceResponseState::ServiceCall(call, send) => { + match call.poll(cx) { Poll::Ready(Ok(res)) => { let (res, body) = res.into().replace_body(()); @@ -273,8 +277,7 @@ where if size.is_eof() { Poll::Ready(()) } else { - *this.state = - ServiceResponseState::SendPayload(stream, body); + this.state.set(ServiceResponseState::SendPayload(stream, body)); self.poll(cx) } } @@ -300,10 +303,10 @@ where if size.is_eof() { Poll::Ready(()) } else { - *this.state = ServiceResponseState::SendPayload( + this.state.set(ServiceResponseState::SendPayload( stream, body.into_body(), - ); + )); self.poll(cx) } }