Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builder::spawn should encode the metadata lifetime contract #76

Open
jstarks opened this issue May 20, 2024 · 3 comments
Open

Builder::spawn should encode the metadata lifetime contract #76

jstarks opened this issue May 20, 2024 · 3 comments

Comments

@jstarks
Copy link

jstarks commented May 20, 2024

Considering this from Builder:

pub fn spawn<F, Fut, S>(self, future: F, schedule: S) -> (Runnable<M>, Task<Fut::Output, M>)
where
    F: FnOnce(&M) -> Fut,
    Fut: Future + Send + 'static,
    Fut::Output: Send + 'static,
    S: Schedule<M> + Send + Sync + 'static,
{
    ...
}

My understanding of the contract around metadata is that it is allocated and pinned as part of task allocation, it is guaranteed to remain valid at least as long as the task's future, and it will not be deallocated without being dropped. If these were not intended to be guarantees, it would be strange to have this extra F wrapper at all, because the caller already had access to the metadata in its old location before it was moved into the task.

However, spawn does not reflect these guarantees in the type system.

I don't think the lifetime guarantee can be encoded with Rust today. You need something like:

F: for<'a> FnOnce(&'a M) -> Fut + 'a

But that doesn't work. Maybe something similar could be done with an explicit trait with GAT and RPITIT, though.

But you can encode the fact that the metadata is effectively pinned for its lifetime. And I think this would be a useful thing to do at the next breaking change:

F: FnOnce(Pin<&M>)
@notgull
Copy link
Member

notgull commented May 21, 2024

I originally intended task metadata that isn't Copy to be most useful in the spawn_unchecked use case. As you mentioned it's difficult to write a safe lifetime in a way that satisfies the proper conditions in Rust's current set of rules.

But that doesn't work. Maybe something similar could be done with an explicit trait with GAT and RPITIT, though.

This might be possible, but we'd have to save it for the next breaking change. I don't want to release smol v3 until futures-core v1.0 is released.

@jstarks
Copy link
Author

jstarks commented May 21, 2024

Yes, that all makes sense. But I want to emphasize that the addition of Pin on the metadata reference will be useful at the next breaking change, even if we can't solve the lifetime issue.

@notgull
Copy link
Member

notgull commented May 23, 2024

From a theoretical point of view I agree, but I'm not sure what Pin<&M> gets us from a practical point of view. &M can be converted to Pin<&M> without any issues, and vice versa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants