smol-rs/async-task

Add blocking wrapper

Closed this issue · 6 comments

zeenix commented

Suggesting we add a blocking::Task, a thin blocking wrapper of Task that's more convenient to use in the blocking code. API:

pub mod blocking {
    pub struct Task<T>(crate::Task<T>);

    impl<T> Task<T> {
        fn cancel(self);
        fn detach(self) -> Option<T>;
        fn wait(self) -> T;
    }

    impl<T> Deref for Task<T> {
        type Target = crate::Task<T>;
        ...
    }

    impl From<crate::Task<T>> for Task<T>;
    impl From<Task<T>> for crate::Task<T>;
}

For the implementation, we will either want to depend on pollster for its block_on or async-io, which IMO would be a bit of an overkill so if we go for async-io, we probably want to feature gate this.

taiki-e commented

I feel that associating this library with specific reactors or executors such as async-io or pollster is not preferable.

  • 👍1
yoshuawuyts commented

I agree that we probably shouldn't add this to async-task, but maybe we can add it to smol or async-std. Also I don't think there's anything really stopping us from adding it as a method to future instead of task: a task is just a special type of future after all. The way I envision this working would be similar to C#'s Task.wait method 1:

trait FutureExt: Future {
    /// Wait for the `Future` to complete execution.
    pub fn wait(self) -> <Self as Future>::Output;
}

C# has a few other overloads for wait; such as timing out after a duration, or timing out based on a message. But I don't think those are needed, since futures are inherently cancellable, and this would be valid:

my_async_function()
    .timeout(Duration::from_secs(3))
    .wait()?;

Footnotes

  1. C# doesn't make the same distinction Rust does between "future" (unmanaged) and "task" (managed). All units of execution are managed, making their tasks equivalent to Rust's tasks. So while we're indeed copying a method which exists on "C# tasks", on our end we need to reinterpret it for our model of async execution. Meaning: I think wait should be available for all futures, not just tasks. ↩

taiki-e commented

@yoshuawuyts
FYI, futures 0.1 had Future::wait method, but it was removed in favor of block_on in 0.2/0.3.

I believe the reason it was removed was because it was associated with the executor (I don't know the exact reason as I was not involved in futures at the time), but I think it is also important to note that it is very easy to incorrectly call such a method and cause a deadlock (rust-lang/futures-rs#2386 (comment))

Also, note that blocking with different executors can also cause deadlocks, even if you were in the right context (e.g., tokio-rs/tokio#4514 -- for this case, it is also related to cooperative scheduling of tokio, but it is probably possible to cause something similar in smol: 1).

yoshuawuyts commented

@taiki-e thanks for the reply! - Those are all great points, thanks for raising them. It seems I have some reading to do before proceeding. Regarding the removal of Future::wait, the following links seem relevant:

yoshuawuyts commented

Aaron summarized the rationale for removing the .wait method from futures here:

The free block_on_all function replaces the wait method on Future, which will be deprecated. In our experience with Tokio, as well as the experiences of other ecosystems like Finagle in Scala, having a blocking method so easily within reach on futures leads people down the wrong path. While it's vitally important to have this "bridge" between the async and sync worlds, providing it as thread::block_on_all highlights the synchronous nature. In addition, the fact that the function automatically blocks on any spawned tasks helps avoid footguns as well.

My main interest in the wait method was because it would allow crates such as flume to be implemented in terms of async Rust, and override their wait method to be more performant, removing the need to have the {recv,send}{,_async} methods. Instead allowing an async -> sync receiver.recv().wait() to be as performant as their current receiver.recv() call.

However there are clearly a lot of issues with a Future::wait method. Some of which may be overcome. But for example, because we would like to enable e.g. rayon and other runtimes to specify the way they handle futures, the wait call would always remain somewhat tricky to implement. This is a lot, and warrants looking at other alternatives.

I think we could achieve a similar effect as a .wait method by specializing thread::block_on. For example: flume could provide its own that can optimize its channel in sync scenarios. And nothing stops other libraries and runtimes from performing similar approaches as well.

Anyway, to conclude: understanding more of the requirements of .wait, and the rationale for why it was removed in favor of thread::block_on, I don't think the benefits of adding it outweigh the downsides. I'd be in favor of closing this issue.

  • 👍1
zeenix commented

Thanks @yoshuawuyts and @taiki-e for your input on this. While I think it'd be great to have this in some shape, I no longer have a use case for it myself so I won't be working on this. just FYI. :)

  • ❤️1