Skip to content

Commit

Permalink
Auto merge of #102737 - RalfJung:poll_fn_pin, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
poll_fn and Unpin: fix pinning

See [IRLO](https://internals.rust-lang.org/t/surprising-soundness-trouble-around-pollfn/17484) for details: currently `poll_fn` is very subtle to use, since it does not pin the closure, so creating a `Pin::get_unchcked(&mut capture)` inside the closure is unsound. This leads to actual miscompilations with `futures::join!`.

IMO the proper fix is to pin the closure when the future is pinned, which is achieved by changing the `Unpin` implementation. This is a breaking change though. 1.64.0 was *just* released, so maybe this is still okay?

The alternative would be to add some strong comments to the docs saying that closure captures are *not pinned* and doing `Pin::get_unchecked` on them is unsound.
  • Loading branch information
bors committed Oct 28, 2022
2 parents 9565dfe + 17d78c4 commit 7174231
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions library/core/src/future/poll_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use crate::task::{Context, Poll};

/// Creates a future that wraps a function returning [`Poll`].
///
/// Polling the future delegates to the wrapped function.
/// Polling the future delegates to the wrapped function. If the returned future is pinned, then the
/// captured environment of the wrapped function is also pinned in-place, so as long as the closure
/// does not move out of its captures it can soundly create pinned references to them.
///
/// # Examples
///
Expand Down Expand Up @@ -41,7 +43,7 @@ pub struct PollFn<F> {
}

#[stable(feature = "future_poll_fn", since = "1.64.0")]
impl<F> Unpin for PollFn<F> {}
impl<F: Unpin> Unpin for PollFn<F> {}

#[stable(feature = "future_poll_fn", since = "1.64.0")]
impl<F> fmt::Debug for PollFn<F> {
Expand All @@ -57,7 +59,8 @@ where
{
type Output = T;

fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<T> {
(&mut self.f)(cx)
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<T> {
// SAFETY: We are not moving out of the pinned field.
(unsafe { &mut self.get_unchecked_mut().f })(cx)
}
}

0 comments on commit 7174231

Please sign in to comment.