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

Feature: generalise Future to accept parameter to poll() #316

Closed
Diggsey opened this issue Dec 29, 2016 · 10 comments
Closed

Feature: generalise Future to accept parameter to poll() #316

Diggsey opened this issue Dec 29, 2016 · 10 comments

Comments

@Diggsey
Copy link
Contributor

Diggsey commented Dec 29, 2016

An executor may wish to pass state into a future each time it is polled. This allows the implementation to exploit the fact that only one future at a time is running on the executor, by having the executor pass in a mutable borrow to some shared state.

This new trait would have all of the same combinators as Future, but would have an extra type parameter determining the type of the state passed to its poll method.

Currently the workarounds are to either use an Rc<RefCell<T>> bound to each future, or a scoped thread-local value, both of which introduce unnecessary overhead.

Another possible extension would be to allow returning data from poll() in the NotReady case, but I don't have a use-case for that.

Somewhat related to #129

@Diggsey
Copy link
Contributor Author

Diggsey commented Dec 29, 2016

This corresponds to returning values from yield in the coroutines RFC: https://github.com/vadimcn/rfcs/blob/coroutines2/text/0000-coroutines.md

@Rufflewind
Copy link
Contributor

I think this is a really useful idea. It would be nice to be able to share state between futures without having to worry about panics due to improper borrowing. I think this would result in Future gaining a 3rd parameter though, which may increase complexity.

@alexcrichton
Copy link
Member

I believe this is a dupe of #129, so closing in favor of that.

@Diggsey
Copy link
Contributor Author

Diggsey commented Mar 7, 2017

@alexcrichton This is different from #129

#129 Is about passing the Task into Futures - I don't imagine this would be an additional type parameter, it would likely be tied to the Task type directly.

This issue is a feature suggestion, which is independent of that issue: generalising the Future trait, to give a new trait, which has an additional associated type, and takes an additional parameter (passed by mutable reference) to its poll method (if #129 is accepted, that would be in addition to a task handle) which allows additional state to be passed in each time the future is polled.

Futures would be interchangeable with this trait when the associated type is unit/empty tuple.

@alexcrichton
Copy link
Member

Ok

@alexcrichton alexcrichton reopened this Mar 7, 2017
@carllerche
Copy link
Member

I don't think this being worth the breaking change for the following reasons:

  • I don't see a compelling use case. There may be one, if so could someone provide an example.
  • This would have a significant hit to the combinators.
  • This can be done out of tree.

As such, if somebody cares about this, I would suggest experimenting in another crate and reporting back.

@Rufflewind
Copy link
Contributor

It would be useful for sharing a &'a mut between all the futures (even ones that are running concurrently). The lifetime prevents it from being stored as a task- or thread-local.

@mdg
Copy link

mdg commented Sep 26, 2017

my case for the use case

Not being able to do this is pretty limiting for futures. It means that a single piece of data cannot own multiple sockets. The multiple sockets have to own that single data and we know how rust likes data having multiple owners.

I can see how that works for a server, but it doesn't seem to fit the case of a client that is using heterogeneous IO with incompatible future types that are started at different times. This is a pretty common scenario. RefCell is a work-around, but then we're giving up all the compile time checks that make Rust so great and we're practically back to checking for null pointers like troglodytes.

Please correct me if I'm wrong, I would love to be, but I've been working with it for a while and just don't see the way to do it without violating borrowing and ownership checks.

as for interface

In my scenario, the shared state is global for the Core. That would put the type is in the Core and the Handle. The current scenario that we have, where there's no state, would just be a specialization, like Core<()>. Then when you want shared state, it would be Core<Whatever> and Handle<Whatever> and TcpStream<Whatever>. We could even make it more like StatefulTcpStream<Whatever> and leave type TcpStream = StatefulTcpStream<()>; In this case, it shouldn't break the combinators b/c they'll all be the same type for the entire core.

@taiki-e
Copy link
Member

taiki-e commented Aug 29, 2019

I think this was fixed in 0.3 (std::future::Future).

@Nemo157
Copy link
Member

Nemo157 commented Aug 30, 2019

Yeah, this is now out of futures-rs' hands since std defines the core trait. It does take a Context object, but there is currently no way for an executor to put its own data onto this object, if there are good use cases for extending this they'll have to be raised as a Rust RFC.

@Nemo157 Nemo157 closed this as completed Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants