rust-lang / futures-rs

Zero-cost asynchronous programming in Rust
https://rust-lang.github.io/futures-rs/
Apache License 2.0
5.36k stars 620 forks source link

Differences between channels in futures and std #840

Closed ghost closed 4 years ago

ghost commented 6 years ago

I'm wondering whether there are any high-level rules driving the design of channels in futures. At first sight, it might seem that futures::sync::mpsc and std::sync::mpsc are trying to mirror each other - the former channels work with asynchronous tasks and the latter channels work with threads. Other than that, they look very similar.

Closing/disconnection

But then there are some peculiar differences. For example, channels in futures have Receiver::close. If this method is useful, is there any reason why channels in std don't have it, too? Or, put differently, was this method added because it fits into a certain model of how futures should be used, or just seemed like a nice addition at a certain time?

Unbounded channels

Another difference is that futures doesn't have unbounded (sometimes called asynchronous) channels, while std does. Is there a reason for that? Golang considers unbounded channels dangerous because they don't provide backpressure. Still, they have their uses (e.g. in Servo), hence why we have them in std. Will we ever have them in futures, too?

Capacities of bounded channels

Next, bounded channels in futures have unusual capacities. Each Sender has its own reserved slot in the channel, which means the actual capacity of a channel is the initial capacity + the number of active senders. There are technical reasons for this - I believe capacity behaves like that simply because it's hard to make it constant without sacrificing performance. These technical hurdles can be overcome, however. In any case, it can be surprising to discover this behavior.

Learnability

There's also a case to be made for learnability. If the Rust book teaches inter-thread communication using std::sync::mpsc, are newcomers not going to expect channels in futures to behave the same way? But there sure seem to be a few important differences, or 'catches' one has to be aware of.

I'm personally a fan of thinking of channels in futures and std as virtually the same, except for what kind of threading/task mechanism they operate on. Or maybe making these channels as similar as possible is simply not a goal we should pursue?

kamyuentse commented 6 years ago

It's that the Unbounded channels you mentioned?

https://github.com/rust-lang-nursery/futures-rs/blob/master/futures-channel/src/mpsc/mod.rs#L376

ghost commented 6 years ago

@kamyuentse Oh, I wasn't aware of the new unbounded channels because they don't show up on docs.rs/futures yet. Never mind about the lack of unbounded channels then. :)

kamyuentse commented 6 years ago

@stjepang I just notice it when I check the code before. Thanks for your great conclusion about the difference.

stsydow commented 5 years ago

I use futures 0.1.25 and got an unexpected panic from the channel. The scenario is sending from a stream (tokio runtime) to a GUI std::thread which calls poll on the Receiver. If all Senders (and their runtime?) is dropped, poll() panics on the receiver - I would expect an Error.

Am I doing it wrong? I found no way to have futures::::Sender with std::::Receiver.

Nemo157 commented 5 years ago

@stsydow if you have a stack trace it would be good to open a new issue for it. Skimming the code I don't see any way that closing the channel should cause a panic to occur.

stsydow commented 5 years ago

Ok, I just debugged a bit further and missed some Details:

I use tokio TcpStream, do different transformations build a bounded channel to send events to a piston GUI thread (which has raw pointers inside and can therefore not run under a normal executor).

I call poll() on the Sender without the proper executor context - as the docs state for poll(). But I can not use wait() because the GUI has its own incompatible event loop.

Everything works until a Sender has an error i.e. when the connection is lost, which is my test case to reproduce this.

Is there a way to construct a proper executor context for poll() and still call it nonblocking? In futures 0.3 seems to be try_next(), but I need a TcpStream from tokio.

Here is the stack trace any way:

thread '<unnamed>' panicked at 'no Task is currently running', /home/st/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:44:9
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:476
   5: std::panicking::begin_panic
             at libstd/panicking.rs:410
   6: futures::task_impl::with
             at /home/st/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:44
   7: futures::task_impl::current
             at /home/st/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:115
   8: <futures::sync::mpsc::Receiver<T>>::try_park
             at /home/st/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/sync/mpsc/mod.rs:902
   9: <futures::sync::mpsc::Receiver<T> as futures::stream::Stream>::poll
             at /home/st/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/sync/mpsc/mod.rs:936
  10: metafly::graphics::MapWindow::update
             at src/graphics.rs:171
  11: metafly::graphics::MapWindow::process_event
             at src/graphics.rs:308
  12: metafly::graphics::MapWindow::event_loop
             at src/graphics.rs:320
  13: metafly::graphics::display::{{closure}}
             at src/graphics.rs:354
thread 'main' panicked at 'can't join display thread!: Any', libcore/result.rs:1009:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:476
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   6: rust_begin_unwind
             at libstd/panicking.rs:325
   7: core::panicking::panic_fmt
             at libcore/panicking.rs:77
   8: core::result::unwrap_failed
             at libcore/macros.rs:26
   9: <core::result::Result<T, E>>::expect
             at libcore/result.rs:835
  10: mfsim::main
             at src/mfsim.rs:191

...

Process finished with exit code 101
bstrie commented 4 years ago

I'm reviewing issues that are blocking the 0.3 milestone and I'd like a contributor to weigh in on the status of this issue. I'm unclear what action would be needed to resolve this, or whether it's still relevant since its original filing.

cramertj commented 4 years ago

I don't think this issue is still relevant-- if there're more changes wanted here, please file an updated issue. Thanks!