Safety of structurally unpinned closures

language design Unsafe Code Guidelines
1

I am trying to implement some generic futures that take ownership of closures in the std (futures 0.3) API. Looking at the source of stream::ForEach as an example of such types in futures-preview, I see that it exempts the inner closure from structural pinning. This is attested by its Unpin impl:

impl<St, Fut, F> Unpin for ForEach<St, Fut, F>
where St: Stream + Unpin,
      F: FnMut(St::Item) -> Fut,
      Fut: Future<Output = ()> + Unpin,
{}

Here is an example that, on the surface, suggests that the Pin guarantees might be violated by ForEach:

    let mut prefix = Some("Hello".to_string());
    future::ready("world".to_string())
        .into_stream()
        .for_each(move |s| {
            let p = prefix.take();
            future::lazy(move |_| {
                let _ = (p, s);
            })
        })

Here, a value captured by the closure given to for_each is moved out of the closure data to be put elsewhere into the state of the ForEach struct. Considering that the entire ForEach instance is pinned when it happens, can it be a soundness violation to move a part of its internal state like that?

2

ForEach has been provided the guarantee that it is pinned, it is up to it what it does with this guarantee. At no point does it propagate this guarantee to the F it contains (and even if it did closures do not propagate pinning to their closed-over variables so there would be no way to use it). Since ForEach does not internally rely on F being pinned and it does not publicly declare that it is via exposing a Pin<&mut F> outside its “unsafety boundary” then it has no obligation to treat the memory containing the F as pinned.

1 Like
3

I see. F, in turn, cannot safely access mutable references (outside of Pin) to any other part of the ForEach value that contains it, so calling the closure cannot break the otherwise pinned value. I don't think, however, that a Pin<&mut Self> method recipient is generally free to expose any of its members under unpinned mutable references.

4

I agree with @Nemo157. This is described in some more detail in the pin module docs.

In particular, in the code you linked, notice

    unsafe_pinned!(stream: St);
    unsafe_unpinned!(f: F);
    unsafe_pinned!(future: Option<Fut>);

This indicates that when a ForEach is pinned, the stream and future fields are also pinned but the f field is not. It is legitimate to declare that a field of a struct is not pinned even if the struct is; in that case you have to make sure to never create a Pin<&[mut] Field> to that field. From what I can see, that is indeed the case here. The only use of f is (self.as_mut().f())(e), and that just needs an &mut F.

This is subtle, but it is also a general pattern for pinning, which is likely why people familiar with it don't document it every single time.

Indeed, this has to be coordinated with every other method of that type. You have to consistently consider the field as either inheriting pinning or not.

5

Thank you. It was not clear to me that exposing any member, even a potentially immovable one, as structurally unpinned is legitimate if done consistently.

6

You may have a look at this users thread and the referenced Stack Overflow question.

7

This topic was automatically closed 540 days after the last reply. New replies are no longer allowed.