actix/actix-net

can call blocking only when running on the multi-threaded runtime

antonio-antuan opened this issue · 7 comments

Hi!

I'm trying to use actix-web 4.0.0-beta5 and actix-rt 2.2.0 and get can call blocking only when running on the multi-threaded runtime when tokio::task::block_in_place called from handler.

Code:

use actix_rt::System;
use actix_web::middleware::Logger;
use actix_web::{web, App, HttpServer, Responder};
use std::sync::Arc;
use tokio::task;

fn main() {
    System::with_tokio_rt(|| {
        tokio::runtime::Builder::new_multi_thread()
            .worker_threads(2)
            .enable_all()
            .build()
            .unwrap()
    })
    .block_on(async_main())
    .unwrap();
}

async fn handle() -> impl Responder {
    task::block_in_place(move || {
        eprintln!("into blocking");
    });
    "hello"
}

async fn async_main() -> std::io::Result<()> {
    let server = HttpServer::new(move || App::new().route("/", web::get().to(handle)));

    server.bind("0.0.0.0:8090")?.run().await
}

cargo.toml:

[package]
name = "rt-test"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
actix-rt = "2.2.0"
tokio = {version = "1", features = ["rt-multi-thread"]}
actix-web = "4.0.0-beta.5"

When task::block_in_place wrapped with System::current().arbiter().spawn( async {...}) response returned but error occured anyway:

async fn handle() -> impl Responder {
    System::current().arbiter().spawn(
        async {
            task::block_in_place(move || {
                eprintln!("into blocking");
            });
        }
    );
    "hello"

The error is pretty clear isn't it?

actix-web does not run in multi-threaded runtime. System is mostly a separate place from most part of actix-web and the only way to hook up to it is with arbiter methods.

The thing about arbiter is spawn still gives you single-thread context by default. You need another tokio::spawn to get into the context of the runtime you passed to System builder.

You can use this example as reference. Basically wrap anything you want to call to multihtread tokio in another tokio::spawn.

Ok, more real example: I have a web app with actix-web, diesel and tokio-diesel. As you may know tokio-diesel uses task::block_in_place under the hood.
The code looks like this:

async fn handle(db_pool: Data<Pool>) -> Result<Json<Vec<Foo>>>:
    some_table::table.select((some_table::column1, some_table::column2)).load_async(&db_pool).await?;

Here is inside load_async call task::block_in_place is used. How do I need to wrap that query if I do not want to receive the error? example shows only one way: I need to use oneshot to pass results outside of arbiter().spawn(...).

Is there any other ways to do it?

Using oneshot channel is fine.

The only problem is everything would be Send bound which is the point of using block_in_place. But that's the price to pay with communicating to a foriegn runtime from your handler to System arbiter as they are actually two runtimes on different threads.

Some like this would work but as you can see the closure you pass to block_in_place must be Send.

fn block_in_place<F, O>(f: F) -> impl std::future::Future<Output = Result<O, actix_web::Error>>
where
    F: FnOnce() -> O + Send + 'static,
    O: Send + 'static,
{
    let (tx, rx) = tokio::sync::oneshot::channel();

    actix_web::rt::System::current().arbiter().spawn(async {
        let res = tokio::spawn(async {
            tokio::task::block_in_place(f)
        }).await.unwrap();

        let _ = tx.send(res);
    });

    async {
        rx.await.map_err(actix_web::error::ErrorInternalServerError)
    }
}

I see.
Thanks for your help.

You are welcome.

It's actually could be useful to make Arbiter::spawn return JoinHandle to save you from the oneshot channel. It does not is mainly a back compat reason I suppose. But I feel it could be a worthy break change.

I think it would be more... elegant not to know about that details inside handlers functions and not to use oneshot. Maybe handlers can be wrapped with Arbiter::spawn and oneshot under-the-hood? Looks like it may be very useful but also I think that it may produce overhead. So it need to be configurable. Maybe something like that:

web::get().to_wrapped_spawn(handle)

In that case there is no need to call System::current().arbiter().spawn(async {tokio::spawn(async move {...})...}} and interact with oneshot directly.

Of course it's just a suggestion and I'm not insist on it.
If you think that it's a good idea I can try to make PR.

My idea was to return a JoinHandle in the form of like actix_rt::spawn(async { rx.await.unwrap() }). This would have even more overhead than using a oneshot directly but would have the same return type as actix_rt::spawn and tokio::spawn. Making the API more cleaner.

Issue and/or PR are welcomed. We can discuss with other contributors and users to decide if it's a worthy change.