park/unpark are tricky to use

[aidanhs]

Quote from #129 (comment), brought here since it's a bit of a tangent. Related to #77

If a user receives a would-block error, they are responsible for figuring out how to get notified when readiness changes.

1. The documentation says this (in fairly assertive language - "In this situation the future will also register interest of the current task in the value being produced."), but it appears the example proxy server doesn't bother parking or registering anything! In fact, being lazy like this generally works...with one notable exception - wait_{future,stream} on Spawn (which will hang forever unless you've done it by the book).
2. The implementation of parking/unparking is pretty invasive. Let's say I have a future I want to call with .poll() so it can be a small cog in a larger machine - but futures may not work unless they're in Task's so I spawn it, but now I need to call poll_future with an unpark argument (though from what I can gather, this can just be a no-op implementation).
3. With a naive implementation of a polling method, park shoots you in the foot - you need to have readiness checks in poll anyway (for first time you call it at minimum), but because of NotReady you're forced to separate out (or duplicate) this logic and either block on it in a thread or dispatch another async task to check for readiness (except the latter one won't work unless you're in an event loop and can grab a reference to it - if someone has called Spawn::wait_* you must spawn a thread I think? Which seems like a very difficult api to correctly behave under). Admittedly, this can be very efficient (e.g. a single async task to accumulate and check for readiness on all tcp sockets you have open), but it's also additional complexity you can't opt out of.

I somewhat like the old way with poll and schedule, except with schedule returning ok, err or notimplemented.

My key problem is that right now it's not easy to implement futures or streams on your own types (part of the problem being that callbacks are troublesome in rust). It's not an easy problem to solve, but I'm not convinced that making everyone have a go is a good idea. In fact, unless I'm gravely misunderstanding (never a remote possibility), even tokio-core doesn't get it right! In theory the following example should exit immediately after the first connection, in practice it just hangs forever. Strace shows it hung in futex (because the thread has parked without a wakeup being registered), whereas https://tokio-rs.github.io/tokio-core/tokio_core/index.html sits in epoll_wait.

extern crate futures;
extern crate tokio_core;

use std::net::SocketAddr;

use futures::task;
use tokio_core::net::TcpListener;
use tokio_core::reactor::Core;

fn main() {
    let addr = "127.0.0.1:8080".parse::<SocketAddr>().unwrap();
    let l = Core::new().unwrap();
    let handle = l.handle();
    let socket = TcpListener::bind(&addr, &handle).unwrap();
    let mut spawn = task::spawn(socket.incoming());
    spawn.wait_stream();
}

[aidanhs]

Related: #131 (comment)

When we chatted about this on IRC, however, the thought was that there'd perhaps be a polling method on Spawn which didn't require a Arc. You'd call it with the intention of "I'll arrange myself to get notified later" via some other means.

(though I'm a little unclear on "I'll arrange myself to get notified later")

[carllerche]

I haven't had time to digest everything, but in your example, it doesn't look like Core was actually started, so I wouldn't think that any connection polled from socket.

[aidanhs]

That's a fair point, but it sort of gives evidence to the thought that not all futures are equal, you need to know how they work instead of relying on the trait to sort it out. I'll be honest, I've no idea how you could use that method in the context of tokio given the need for a Core.

Also, minor correction to my post: "(though from what I can gather, this can just be a no-op implementation)" - this isn't true, it actually needs to unpark the correct task.

[alexcrichton]

Thanks for the report! There's a few points I want to help clear up first, and I think that may help?

The implementation of #77 has rationale for why it was implemented, but to reiterate:

• It's far more ergonomic to not have both schedule and poll. This can also be a perf benefit because you don't have to navigate the state machine in both locations. Finally, it's less error prone because you only define your logic once.
• Not requiring a Task to be passed around enables implementations of standard traits like Read and Write. They can automatically register interest in the current task on a read/write byte stream without any extra legwork.

In general it's not expected that you call poll on a future directly. Rather you'll spawn it into another form of executor like a thread pool, an event loop, or via wait. Note that wait is particularly dangerous though as it assumes the value is being produced on another thread (e.g. doesn't need this thread's resources to complete), and this isn't always true.

With that in mind...

but it appears the example proxy server doesn't bother parking or registering anything

Ah this is actually the fun part! The implementation there does indeed register interest on underlying events as part of the calls to poll_read and poll_write. If these functions return NotReady, then the task is parked to receive a notification when they are ready.

The contract of poll is that if it returns NotReady the current task is scheduled to receive a notification when it may otherwise return Ready. If this contract is not fulfilled then yes, you'll get a deadlock, but generally bad things happen when contracts are not followed!

The implementation of parking/unparking is pretty invasive.

I think this depends on your perspective and when you're mentioning poll who's calling it. I'd actually claim that it's very uninvasive because it's all happening behind the scenes. When implementing all you need to do is look for actual events (I/O, messages, other futures, etc). If any of them return NotReady you're scheduled to receive a notification, so you can just propagate that upwards.

For poll, it is indeed difficult to call Future::poll directly, and you are indeed supposed to call task::spawn first. The poll_future method takes an Arc<Unpark> argument so you can know when to attempt to poll the future again. This is generally provided by some form of executor, be it a thread pool, an event loop, or wait. The emphasis here though is that the implementation here is provided by a stock implementation of an executor rather than custom all the time. It's generally a sign that something's gone wrong if you find yourself calling poll_future just to extract the result.

With a naive implementation of a polling method, park shoots you in the foot

I don't think though that anything is necessarily implied about when/where events have to happen. You don't, for example, have to spawn a thread if you return NotReady. You simply, as a future, just need to ensure that a notification (unpark) is sent when the value is ready. This may be done by performing work on the current thread as well.

If a future has wait called on it, then that's an assertion made by the caller that the future will indeed complete on another thread. This assertion unfortunately can't be checked, and isn't always true (like I mentioned above), but there are legitimate cases for wait regardless.

My key problem is that right now it's not easy to implement futures or streams on your own types (part of the problem being that callbacks are troublesome in rust).

I'd be very interested to drill more into this! It's very much the intention of this crate that it is easy to implement Future and Stream, as this is very much expected! This may indicate a severe lack of documentation, and I'd love to know where to improve!

In theory the following example should exit immediately after the first connection

As @carllerche mentioned the flaw here is that the event loop isn't running. The future will be completed on the event loop, owned by the current thread, so .wait() isn't valid to call here. In general wait is very dangerous to call on arbitrary futures and needs to be a careful consideration to ensure that the future will always complete on another thread.

Here you'll want to do l.run(socket.incoming().into_future()) which will run the event loop while it's waiting for the future to resolve. This means that the future is resolved concurrently with the rest of the event loop's tasks.

---

Does all that make sense! I'd love to get to the bottom of this and see what the key areas are that we need to improve, clearly something's tricky!

[aidanhs]

First off, thanks for the comprehensive response. The proxy example is very sneaky, thanks for pointing that out :) You've also cleared up a significant misunderstanding of mine.

In general it's not expected that you call poll on a future directly. Rather you'll spawn it into another form of executor like a thread pool, an event loop, or via wait. Note that wait is particularly dangerous though as it assumes the value is being produced on another thread (e.g. doesn't need this thread's resources to complete), and this isn't always true.

I think the documentation needs to be more forward about mentioning event loops/executors. These are crucial building blocks and I hadn't realised! In particular, you say "You simply, as a future, just need to ensure that a notification (unpark) is sent when the value is ready." - taking this sentence in isolation is very confusing ("how can I ensure a notification is sent? what if I don't own the calling code?") until you realise that there's an implicit expectation of having your own event loop/executor that you can stash wakeup conditions in to make sure they get checked every now and again. tokio is a great example - it provides futures than in theory magically register themselves for wakeup on calling poll...but actually there's a dependency on having the reactor core running.

The documentation says the following about futures: "The poll method is not intended to be called in general, but rather is typically called in the context of a "task" which drives a future to completion." Actually, this seems like it needs to say: "It is not possible to call the poll method in general, as Futures may rely on specific event loop implementations being run to register notifications correctly." - the important change is the implication that it may actually a bug to call poll directly, depending on the implementation.

"In general wait is very dangerous to call on arbitrary futures and needs to be a careful consideration to ensure that the future will always complete on another thread." - this probably needs calling out better as well.

---

My misunderstandings aside, I had a think about "My key problem is that right now it's not easy to implement futures or streams on your own types (part of the problem being that callbacks are troublesome in rust)." and came to a realisation about why I'm finding it tricky - I'm trying to use futures at a lower level than they're intended to be used at. Specifically, I just want to be able to call poll on a future, and if it's not ready then I'll come back later. I'd also be fine with a requirement to wrap a bunch of futures in a task and call poll on that - the crucial aspect is that these futures are self-contained and can easily be plugged into an existing application that isn't built around tokio/futures-rs.

Because the of current contract (a notification will be registered) the thing that's actually hard (contrary to me saying it's creating futures and stream is general) is creating a self-contained future that doesn't require additional machinery (like the tokio reactor core) to support notification. This is why the implementation of tokio-rs/tokio-core#24 will never return NotReady - I want to be able to call it from anywhere. Just to acknowledge: I (now) recognise that requiring the additional machinery is actually a design choice.

In truth, additional machinery isn't fundamentally a bad thing - having all tokio futures registering their interest via the core means you only need one call to epoll. But it does mean that the Future trait says very little when trying to use futures, because you need to understand how notifications get registered for a specific future implementation in order to use it properly - in the case of tokio, that understanding manifests as "make sure you run the reactor core". It does happen behind the scenes, but you still need to be aware of how it's working or you get misunderstandings like my example above.

If any of them return NotReady you're scheduled to receive a notification, so you can just propagate that upwards.

This is true. The crucial question for me is how you implement this at the edges, i.e. where you initially generate a NotReady. That's why I talk about the machinery above - you need it to be present and running to make sure you can reliably schedule a notification.

I'd be very interested to drill more into this! It's very much the intention of this crate that it is easy to implement Future and Stream, as this is very much expected.

I think this is covered above, but to state explicitly: my issue was that I didn't want to use the reactor core for the IO I was doing, but if I don't use it then I have to implement my own event loop or it becomes painful to handle notifications.

The emphasis here though is that the implementation here is provided by a stock implementation of an executor rather than custom all the time.

Sorry, I'm not clear what this sentence means. I understand the surrounding sentences, but this one has me confused!

---

Again, thanks for the response - I've got a much better understanding of how things are meant to work together now. I'm not sure I 100% agree that notifications should be compulsory (it'd be nice to be able to opt-out), but I can see there would be complexity involved.

Aside from documentation being more forward about both event loops and futures possibly implicitly depending on other things, I think I'd like to see better treatment for applications that only want part of the application to use tokio/futures. tokio-rs/tokio-core#24 is a step towards this, but it makes me a touch uncomfortable that there's no way to express "this will never return NotReady, it's just a way to turn the event loop". Potentially it's trying to use the wrong abstraction. I'll leave a couple of thoughts on there.

[alexcrichton]

Oh man, lots to digest here!

About the documentation bits, I'd definitely love to improve wherever possible. One caveat though is that I don't think we should document poll as "you should never call this" because if you're implementing poll itself it's perfectly ok to call other poll methods (for example). At the top level, though, yeah it shouldn't be called.

It's also definitely our intention that futures can work with any form of backend, whether it be tokio-core or not. Additionally, it should slot in if possible to existing applications! Which is to say, friction here is definitely not intended :)

Note that the "machinery" here is indeed all employed only at the edge nodes, it rarely happens in the futures crate itself except for oneshot/channel implementations. The Spawn<F> type requires a Arc<Unpark> argument for how to deliver readiness notifications, but you can of course provide a noop if you're not using that.

Put another way, this "extra machinery" if you really don't want to use it should be optional. Yes if you don't use a tokio-core event loop you'll have to write your own, but it sounds like that's the use case you're tackling here?

The emphasis here though is that the implementation here is provided by a stock implementation of an executor rather than custom all the time.

Sorry, I'm not clear what this sentence means. I understand the surrounding sentences, but this one has me confused!

Oh I only meant to say that most uses of futures will end up using a stock implementation of executors like CpuPool or tokio_core::reactor::Core. That way you never have to worry about calling poll at the top-level as it's all handled for you. That's not to say that custom implementations aren't allowed, they're just not as ergonomic

[aidanhs]

It's also definitely our intention that futures can work with any form of backend, whether it be tokio-core or not. Additionally, it should slot in if possible to existing applications! Which is to say, friction here is definitely not intended :)

Note that the "machinery" here is indeed all employed only at the edge nodes

At present the tokio reactor takes over an entire thread when you want to start it - there's no way to 'cooperatively' run the reactor alongside other things. Hence the turn method. I've been waving my hands a bit, so here's a concrete example.

I've got an application that runs code every 'tick', using epoll and the like from before futures existed, along with other async libraries that don't have any async notification method at all. Here's a simplified example for illustration purposes:

let mut in_q = vec![];
let mut out_q = vec![];
let mut state = ...;
loop {
    custom_network_stack.async_recv(&mut in_queue);
    custom_network_stack.async_send(&mut out_q);
    handle_messages(&mut state, &mut in_q, &mut out_q);
    handle_tcp_connections(&mut state);
}

I'd like to convert part of this application (handle_tcp_connections) to use futures. The crucial point is that at the point of wanting to do this I'm unable to meet the requirement that a NotReady result in my custom network stack will schedule something to happen when it's ready - perhaps this isn't something I have time to do right now, perhaps this isn't something that's possible to do. I therefore want to poll any tcp connection futures on each iteration of my main loop, but leave the rest of the existing async code alone.

However, I can't do this because tcp connection futures require the presence of the tokio reactor core machinery to wake up and continue executing when they're ready...but the tokio reactor core currently doesn't allow you to use it at all without taking over the whole thread.

ssh2-rs is a possible useful example here. Sessions may be async, which means I can create async channels and use them in an async application. Unfortunately, even if I did all the work to ensure plumb the underlying tcp stream into the tokio reactor core, it wouldn't be ideal - libssh2 supports multiple channels interleaving, but the event would fire for any channel receiving data. Instead of using this somewhat broken abstraction, I'd prefer to just call the ssh2-rs methods myself and run a reactor core alongside for any 'normal' tokio-supported futures.

[alexcrichton]

Hm so this issue feels pretty far afield of why it was first opened at this point. It seems like your last comment boils down to ".run blocks and that's why turn was added", but there's some important points of note I think:

• You don't know when to come back to the reactor. If futures aren't ready you don't know when they'll be ready and otherwise just can't correctly proceed.
• We could force run(futures::done(Ok(())) to turn the event loop, being the equivalent of turn.

I think it'd be worth though coming back to the original points. Is this issue still value? Should this stay open?

[aidanhs]

You're right, I've meandered off topic.

Coming back to park/unpark, probably the only thing that needs doing is "I think the documentation needs to be more forward about mentioning event loops/executors." Since I'm the one confused, I'll do this at some point, mentioning that "event loops are the expected way to register notifications" in a few places. I might also look at writing a tutorial on writing an event loop, because I haven't seen one and think it'd be very useful for people wanting to understand the internals. I don't mind if this issue is open or closed in the meantime.

I'll respond to your two points in the PR since it's more directly related.

[alexcrichton]

Ok, awesome! I don't mind leaving this open in the meantime to track documentation. I'd love to have a tutorial on writing an event loop, it sounds like a great idea!

[WaDelma]

After reading this I am really confused. So if I am not supposed to call poll directly on future how I am supposed to ignore futures that are not ready yet?

To give background information:
I am changing a voxel game engine prototype that was using different future library to use this one.
Basically I have map from chunk coordinate to enum that is either future or a value the future has resolved to.
Each tick I am using spawn_fn to create CpuFutures which tries first to load chunk and if that fails generates one and inserts it to the map.
When rendering I want to iterate through all fully loaded chunks so I created an iterator that uses poll to check if future is ready.
This panics with "no Task is currently running".

This all worked fine with the other library that I was using... So how should I be doing this?

[alexcrichton]

@WaDelma working at the boundaries of futures is unfortunately not the most ergonomic right now. If you're not running inside of another future or if you're responsible for running futures yourself it's somewhat tricky to ensure everything works out.

The task that's iterating through all the chunks, is that not itself a future? One that's resolved once all the chunks are loaded? The essence of the solution will be to push the boundary of the future higher to the edges of the system, or if you're feeling intrepid you can dive into using task::spawn

[WaDelma]

Not really... When I iterate through all chunks I want to just render those that are loaded and ignore those that are still on the way. Rendering should never block.

[alexcrichton]

In that case I'd probably recommend a construction like:

• Join all futures with another future
• Use task::spawn on this one future
• Call poll_future

You can then drop the future after that, it'll do as much work as it can internally and then if you're not interested in the rest it'll all get canceled.

[WaDelma]

Well I don't want to cancel the loading either...
After reading "Rust futures at a glance" I actually realised I have to basically implement thread pool myself and insert my stuff into it to make this work because the game loop is basically an event loop.

I'll just go back to using the other library.

[M2Ys4U]

I, too, am struggling to understand how I can use futures because of this issue. When I read that the future will "register interest of the current task in the value being produced" when NotReady was called I understood that to mean that happened as a matter of course. Obviously this isn't the case and one has to manually do this if one is to implement Future.

My situation: I'm accepting new TCP connections in one future and create a wrapper struct to do some buffering on them. I push those wrappers though a channel to another future. This second future's poll method receives those connections and calls poll on each of them in turn to pull out messages. Both of these futures are executed using tokio.

If I return NotReady then my future isn't called again. If I return Ready(_) then the CPU ends up spinning at 100%.

I have no idea how I'm supposed to call park and unpark to arrange for the Task to be woken back up given the only way for me to know would be to call poll on the owned wrappers, which is what the future's own poll method does.

[alexcrichton]

We now have extensive documentation at both https://tokio.rs/docs/going-deeper/futures-model/ and https://tokio.rs/docs/going-deeper/tasks/, so I'm going to close this.

[cwaldren]

The link to futures-model above is dead; I believe it should point here to help future people like me who are still struggling to understand futures!