When is it safe to move a member value out of a pinned future?
Asked Answered
N

2

26

I'm writing a future combinator that needs to consume a value that it was provided with. With futures 0.1, Future::poll took self: &mut Self, which effectively meant that my combinator contained an Option and I called Option::take on it when the underlying future resolves.

The Future::poll method in the standard library takes self: Pin<&mut Self> instead, so I've been reading about the guarantees required in order to safely make use of Pin.

From the pin module documentation on the Drop guarantee (emphasis mine):

Concretely, for pinned data you have to maintain the invariant that its memory will not get invalidated from the moment it gets pinned until when drop is called. Memory can be invalidated by deallocation, but also by replacing a Some(v) by None, or calling Vec::set_len to "kill" some elements off of a vector.

And Projections and Structural Pinning (emphasis mine):

You must not offer any other operations that could lead to data being moved out of the fields when your type is pinned. For example, if the wrapper contains an Option<T> and there is a take-like operation with type fn(Pin<&mut Wrapper<T>>) -> Option<T>, that operation can be used to move a T out of a pinned Wrapper<T> -- which means pinning cannot be structural.

However, the existing Map combinator calls Option::take on a member value when the underlying future has resolved:

fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<T> {
    match self.as_mut().future().poll(cx) {
        Poll::Pending => Poll::Pending,
        Poll::Ready(output) => {
            let f = self.f().take()
                .expect("Map must not be polled after it returned `Poll::Ready`");
            Poll::Ready(f(output))
        }
    }
}

The f method is generated by the unsafe_unpinned macro and looks roughly like:

fn f<'a>(self: Pin<&'a mut Self>) -> &'a mut Option<F> {
    unsafe { &mut Pin::get_unchecked_mut(self).f }
}

It appears that Map violates the requirements that are described in the pin documentation, but I believe that the authors of the Map combinator know what they are doing and that this code is safe.

What logic allows them to perform this operation in a safe manner?

Nagpur answered 9/5, 2019 at 11:33 Comment(1)
If you have not already, I would encourage you to make a post on either URLO or IRLO. The question is tricky enough that I would not mind the opinion of experts (withoutboats, notably) to set my mind at ease. I have a feeling that it's safe (Pin only being about stability over suspension points), but...Leduc
J
13

It is all about structural pinning.

First, I will use the syntax P<T> to mean something like impl Deref<Target = T> — some (smart) pointer type P that Deref::derefs to a T. Pin only "applies" to / makes sense on such (smart) pointers.

Let's say we have:

struct Wrapper<Field> {
    field: Field,
}

The initial question is

Can we get a Pin<P<Field>> from a Pin<P<Wrapper<Field>>>, by "projecting" our Pin<P<_>> from the Wrapper to its field?

This requires the basic projection P<Wrapper<Field>> -> P<Field>, which is only possible for:

  • shared references (P<T> = &T). This is not a very interesting case given that Pin<P<T>> always derefs to T.

  • unique references (P<T> = &mut T).

I will use the syntax &[mut] T for this type of projection.

The question now becomes:

Can we go from Pin<&[mut] Wrapper<Field>> to Pin<&[mut] Field>?

The point that may be unclear from the documentation is that it is up to the creator of Wrapper to decide!

There are two possible choices for the library author for each struct field.

There is a structural Pin projection to that field

For instance, the pin_utils::unsafe_pinned! macro is used to define such a projection (Pin<&mut Wrapper<Field>> -> Pin<&mut Field>).

For the Pin projection to be sound:

  • the whole struct must only implement Unpin when all the fields for which there is a structural Pin projection implement Unpin.

    • no implementation is allowed to use unsafe to move such fields out of a Pin<&mut Wrapper<Field>> (or Pin<&mut Self> when Self = Wrapper<Field>). For instance, Option::take() is forbidden.
  • the whole struct may only implement Drop if Drop::drop does not move any of the fields for which there is a structural projection.

  • the struct cannot be #[repr(packed)] (a corollary of the previous item).

In your given future::Map example, this is the case of the future field of the Map struct.

There is no structural Pin projection to that field

For instance, the pin_utils::unsafe_unpinned! macro is used to define such a projection (Pin<&mut Wrapper<Field>> -> &mut Field).

In this case, that field is not considered pinned by a Pin<&mut Wrapper<Field>>.

  • whether Field is Unpin or not does not matter.

    • implementations are allowed to use unsafe to move such fields out of a Pin<&mut Wrapper<Field>>. For instance, Option::take() is allowed.
  • Drop::drop is also allowed to move such fields,

In your given future::Map example, this is the case of the f field of the Map struct.

Example of both types of projection

impl<Fut, F> Map<Fut, F> {
    unsafe_pinned!(future: Fut); // pin projection -----+
    unsafe_unpinned!(f: Option<F>); // not pinned --+   |
//                                                  |   |
//                 ...                              |   |
//                                                  |   |
    fn poll (mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<T> {
        //                                          |   |
        match self.as_mut().future().poll(cx) { // <----+ required here
            Poll::Pending => Poll::Pending, //      |
            Poll::Ready(output) => { //             |
                let f = self.f().take() // <--------+ allows this
Jesus answered 13/5, 2019 at 17:18 Comment(6)
It would be helpful to mention why Map chooses not to have a pin projection to f. Any self-referential F will be invalidated during poll causing UB. I think the answer is that the user observes f being moved into Map upon construction (thus invalidating any internal references) and we don't need to do anything with f until after take is called, so treating f as pinned (and then requiring F: Unpin everywhere) doesn't add anything: T: Unpin only has semantic value when a Pin<P<T>> exists. It would, however, lint if people try to pass a self-referential f into Map.Sweltering
Although I guess the whole point of Pin/Unpin versus (a hypothetical) !Move is that we don't want it to infect all APIs where a move occurs.Sweltering
@Sweltering yes, this is a good example of Pin not being !Move. The only thing that needs to be pinned here is the inner future, wrapped in Map. But given that the only thing actually pinned (when polled) is the outermost wrapper, the pin projection is needed to guarantee the "transitivity" of the Pin applying to the inner future. In all this business the closure is just a good old value safely moved into the structure; it would only be possible for it to be self-referential if it already dealt with it on its own (e.g., behind a pointer (F = Pin<Box<...>>)), as usual.Jesus
Or in other words: worst case scenario, F : !Unpin, which means that once behind a Pinned pointer, the closure cannot move out. But given the lack of Pin projection in the Map case, it is actually never pinned: sitting next to a pinned value does not semantically Pin you; you can always stand up and leave (leaving a Noone [here] note behind :) ).Jesus
Thanks for the answer! I've made some stylistic edits, please check to make sure I haven't lost any meaning. Can you explain why you've grouped "use unsafe to move" underneath the Unpin bullet? What's the specific relation between those two points?Nagpur
When you get a Pin<&mut Wrapper<_>> (and you don't know if it isUnpin), if you want to move anything out of the struct you need &mut _ references and thus need to use unsafe (actually, with interior mutability you could move out using a & _ reference, but it is alsto the reason why wrappers with interior mutability cannot offer structural pinning, thus ruling out the first bullet situation). Hence my use unsafe to move phrasing. Such usage of unsafe is sound only in two cases: when not moving, or when dealing with a non structurally-pinned field.Jesus
S
5

edit: This answer is incorrect. It remains here for posterity.

Let's begin by recalling why Pin was introduced in the first place: we want to statically ensure that self-referential futures cannot be moved, thus invalidating their internal references.

With that in mind, let's take a look at the definition of Map.

pub struct Map<Fut, F> {
    future: Fut,
    f: Option<F>,
}

Map has two fields, the first one stores a future, the second stores a closure which maps the result of that future to another value. We wish to support storing self-referential types directly in future without placing them behind a pointer. This means that if Fut is a self-referential type, Map cannot be moved once it is constructed. That is why we must use Pin<&mut Map> as the receiver for Future::poll. If a normal mutable reference to a Map containing a self-referential future was ever exposed to an implementor of Future, users could cause UB using only safe code by causing the Map to be moved using mem::replace.

However, we don't need to support storing self-referential types in f. If we assume that the self-referential part of a Map is wholly contained in future, we can freely modify f as long as we don't allow future to be moved.

While a self-referential closure would be very unusual, the assumption that f be safe to move (which is equivalent to F: Unpin) is not explicitly stated anywhere. However, we still move the value in f in Future::poll by calling take! I think this is indeed a bug, but I'm not 100% sure. I think the f() getter should require F: Unpin which would mean Map can only implement Future when the closure argument is safe to be moved from behind a Pin.

It's very possible that I'm overlooking some subtleties in the pin API here, and the implementation is indeed safe. I'm still wrapping my head around it as well.

Sweltering answered 11/5, 2019 at 8:6 Comment(4)
Thank you for the answer. I think this is indeed a bug, but I'm not 100% sure — is there some way that you could remove any doubt one way or the other?Nagpur
Putting F: Unpin isn't necessary, because it has an F: FnOnce(Fut::Output) -> T bound, and closures are always Unpin (at least for now). Though putting F: Unpin probably doesn't hurt anything either.Hautesalpes
Actually, I just tested it. It is possible for a closure to be !Unpin if it closes over !Unpin values. So it may indeed be a bug to not require F: Unpin!Hautesalpes
@Pauan: The FnOnce seems to limit the observability here. One of the key points of Pin is to pin a value in memory across suspension points, however even if F closes over a !Unpin value, this value is never accessed across suspension points, it's only accessed once.Leduc

© 2022 - 2024 — McMap. All rights reserved.