Future::spawn should not require 'static

[apoelstra]

There are two 'static bounds on the implementation of Future::spawn. One is that T: 'static, which I think is actually unnecessary. Edit: Actually I think it is, though maybe I'm just not seeing a clever way to avoid it.

The other is that F: FnOnce() -> T + Send + 'static, which can be weakened to a parameter lifetime 'a provided you add a PhantomData marker to Future tying 'a to the lifetime of the Future itself. (I'm assuming here that the Drop impl on Future ends the async thread before completing; is this true?)

[carllerche]

I have not yet done an audit of all the lifetimes (the code predates Send not implying 'static). I would be happy to accept a PR that does some of the work 😄

[apoelstra]

I started work on a PR, then realized that this 'static bound originates in that of std::thread::spawn, which is marked stable. I still think it's morally unnecessary, but unless we can get the bound removed in libstd it might be practically required.

[carllerche]

@apoelstra There may be a way to rewrite Future::spawn to use thread::scoped instead.

[apoelstra]

Oh, awesome @carllerche , I think you're right. I'll try to write such a PR tonight. Right now I am doing some really nasty unsafe trickery to extend the lifetimes of some borrows to 'static (when really they will not outlive the function call!), and I'd like to not be doing this.

And thanks for this library by the way!

[apoelstra]

I think I'm blocked on rust-lang/rust#23992 -- if I add a lifetime parameter to Future, then rustc overflows in future.rs at the trait implementation

impl<'a, T: Send + 'a, E: Send> Async for Future<'a, T, E> {
    type Value = T;
    type Error = E;
    type Cancel = Receipt<Future<'a, T, E>>;
    ...

(You can see that Cancel is defined in terms of Self, hence the bug occurring.)

[carllerche]

Indeed, I've definitely hit a few ICEs working on Eventual.

We should also think about how adding a lifetime will affect the common usage patters of Future. Will the lifetime be elided? Should this be on Future or should there be ScopedFuture (or some other name), etc...

[apoelstra]

My thinking is, it would almost always be elided, and be equal to the shortest lifetime of the things captured in the closure. In practice this would look something like

{
    // set up environment
    // spawn a bunch of Futures whose closures contain references to the environment
    // join all the closures
}

that is, a simple fork-then-join algorithm entirely contained in one block. I think this is the most common use of Future. I think elision will almost always work for this, so the user experience is the same. No need for a second type.

This is what I'm doing in rust-bitcoin right now. In my case the block is a function block and I am using function arguments which I've only borrowed.
https://github.com/apoelstra/rust-bitcoin/blob/08a20f8764810ff1b49ff08e4c9f218d0c2ccfb6/src/blockdata/utxoset.rs#L268-L289
You can see it's not pretty how I'm handling it now :)

The cases where elision wouldn't occur would be like, defining a struct with a Future in it and now you have to put a lifetime parameter on the struct. You can avoid this by explicitly using a Future<'static, T> if you really want to, which IMHO isn't too much of a burden.

[carllerche]

Given rust-lang/rust#24292 I'm going to close this for now. This should probably be recreated in the future once thread::scoped is stable.