From c999699f5ed12d1d1a10f0388cdd0ac745570131 Mon Sep 17 00:00:00 2001 From: Daniel Bloom <82895745+Daniel-Bloom-dfinity@users.noreply.github.com> Date: Tue, 9 May 2023 09:01:57 -0700 Subject: [PATCH] sync: remove 'static bound from `PollSender` (#5665) --- tokio-util/src/sync/mpsc.rs | 59 +++++++++++++++++++++++++++++++------ tokio-util/tests/mpsc.rs | 23 +++++++++++++++ 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/tokio-util/src/sync/mpsc.rs b/tokio-util/src/sync/mpsc.rs index 55ed5c4de..fd48c7258 100644 --- a/tokio-util/src/sync/mpsc.rs +++ b/tokio-util/src/sync/mpsc.rs @@ -44,7 +44,7 @@ enum State { pub struct PollSender { sender: Option>, state: State, - acquire: ReusableBoxFuture<'static, Result, PollSendError>>, + acquire: PollSenderFuture, } // Creates a future for acquiring a permit from the underlying channel. This is used to ensure @@ -64,13 +64,56 @@ async fn make_acquire_future( } } -impl PollSender { +type InnerFuture<'a, T> = ReusableBoxFuture<'a, Result, PollSendError>>; + +#[derive(Debug)] +// TODO: This should be replace with a type_alias_impl_trait to eliminate `'static` and all the transmutes +struct PollSenderFuture(InnerFuture<'static, T>); + +impl PollSenderFuture { + /// Create with an empty inner future with no `Send` bound. + fn empty() -> Self { + // We don't use `make_acquire_future` here because our relaxed bounds on `T` are not + // compatible with the transitive bounds required by `Sender`. + Self(ReusableBoxFuture::new(async { unreachable!() })) + } +} + +impl PollSenderFuture { + /// Create with an empty inner future. + fn new() -> Self { + let v = InnerFuture::new(make_acquire_future(None)); + // This is safe because `make_acquire_future(None)` is actually `'static` + Self(unsafe { mem::transmute::, InnerFuture<'static, T>>(v) }) + } + + /// Poll the inner future. + fn poll(&mut self, cx: &mut Context<'_>) -> Poll, PollSendError>> { + self.0.poll(cx) + } + + /// Replace the inner future. + fn set(&mut self, sender: Option>) { + let inner: *mut InnerFuture<'static, T> = &mut self.0; + let inner: *mut InnerFuture<'_, T> = inner.cast(); + // SAFETY: The `make_acquire_future(sender)` future must not exist after the type `T` + // becomes invalid, and this casts away the type-level lifetime check for that. However, the + // inner future is never moved out of this `PollSenderFuture`, so the future will not + // live longer than the `PollSenderFuture` lives. A `PollSenderFuture` is guaranteed + // to not exist after the type `T` becomes invalid, because it is annotated with a `T`, so + // this is ok. + let inner = unsafe { &mut *inner }; + inner.set(make_acquire_future(sender)); + } +} + +impl PollSender { /// Creates a new `PollSender`. pub fn new(sender: Sender) -> Self { Self { sender: Some(sender.clone()), state: State::Idle(sender), - acquire: ReusableBoxFuture::new(make_acquire_future(None)), + acquire: PollSenderFuture::new(), } } @@ -97,7 +140,7 @@ impl PollSender { State::Idle(sender) => { // Start trying to acquire a permit to reserve a slot for our send, and // immediately loop back around to poll it the first time. - self.acquire.set(make_acquire_future(Some(sender))); + self.acquire.set(Some(sender)); (None, State::Acquiring) } State::Acquiring => match self.acquire.poll(cx) { @@ -194,7 +237,7 @@ impl PollSender { match self.state { State::Idle(_) => self.state = State::Closed, State::Acquiring => { - self.acquire.set(make_acquire_future(None)); + self.acquire.set(None); self.state = State::Closed; } _ => {} @@ -215,7 +258,7 @@ impl PollSender { // We're currently trying to reserve a slot to send into. State::Acquiring => { // Replacing the future drops the in-flight one. - self.acquire.set(make_acquire_future(None)); + self.acquire.set(None); // If we haven't closed yet, we have to clone our stored sender since we have no way // to get it back from the acquire future we just dropped. @@ -255,9 +298,7 @@ impl Clone for PollSender { Self { sender, state, - // We don't use `make_acquire_future` here because our relaxed bounds on `T` are not - // compatible with the transitive bounds required by `Sender`. - acquire: ReusableBoxFuture::new(async { unreachable!() }), + acquire: PollSenderFuture::empty(), } } } diff --git a/tokio-util/tests/mpsc.rs b/tokio-util/tests/mpsc.rs index a3c164d3e..74b83c211 100644 --- a/tokio-util/tests/mpsc.rs +++ b/tokio-util/tests/mpsc.rs @@ -27,6 +27,29 @@ async fn simple() { send.send_item(42).unwrap(); } +#[tokio::test] +async fn simple_ref() { + let v = vec![1, 2, 3i32]; + + let (send, mut recv) = channel(3); + let mut send = PollSender::new(send); + + for vi in v.iter() { + let mut reserve = spawn(poll_fn(|cx| send.poll_reserve(cx))); + assert_ready_ok!(reserve.poll()); + send.send_item(vi).unwrap(); + } + + let mut reserve = spawn(poll_fn(|cx| send.poll_reserve(cx))); + assert_pending!(reserve.poll()); + + assert_eq!(*recv.recv().await.unwrap(), 1); + assert!(reserve.is_woken()); + assert_ready_ok!(reserve.poll()); + drop(recv); + send.send_item(&42).unwrap(); +} + #[tokio::test] async fn repeated_poll_reserve() { let (send, mut recv) = channel::(1);