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

Renaming tokio::main attribute #1564

Closed
vorner opened this issue Sep 17, 2019 · 13 comments
Closed

Renaming tokio::main attribute #1564

vorner opened this issue Sep 17, 2019 · 13 comments

Comments

@vorner
Copy link
Contributor

vorner commented Sep 17, 2019

A follow up on a blog post https://vorner.github.io/2019/09/15/play-with-new-async.html and a discussion in the dev channel.

The #[tokio::main] can actually be applied to any function that doesn't take arguments. This is non-obvious from the name (and the runtime crate explicitly forbids such use). Would it make sense to rename it to eg. #[tokio::entry]?

Furthermore, is there a need for the „doesn't take arguments“ limitation?

Version

0.2.0-alpha.4

└── tokio v0.2.0-alpha.4
    ├── tokio-executor v0.2.0-alpha.4
    │   ├── tokio-sync v0.2.0-alpha.4
    ├── tokio-io v0.2.0-alpha.4
    ├── tokio-macros v0.2.0-alpha.4
    ├── tokio-net v0.2.0-alpha.4
    │   ├── tokio-codec v0.2.0-alpha.4
    │   │   └── tokio-io v0.2.0-alpha.4 (*)
    │   ├── tokio-executor v0.2.0-alpha.4 (*)
    │   ├── tokio-io v0.2.0-alpha.4 (*)
    │   ├── tokio-sync v0.2.0-alpha.4 (*)
    ├── tokio-sync v0.2.0-alpha.4 (*)
    ├── tokio-timer v0.3.0-alpha.4
    │   ├── tokio-executor v0.2.0-alpha.4 (*)
    │   └── tokio-sync v0.2.0-alpha.4 (*)

Platform

Linux hydra 5.2.11-hydra #1 SMP Fri Sep 6 18:50:52 CEST 2019 x86_64 AMD FX-8370 Eight-Core Processor AuthenticAMD GNU/Linux

(but probably irrelevant here)

Subcrates

tokio-macros

@LucioFranco
Copy link
Member

@vorner what about:

#[tokio::run]
async fn main() {}

#[tokio::run]
async fn do_thing() {}

@vorner
Copy link
Contributor Author

vorner commented Sep 18, 2019

That would probably work too. Still, it would be nice to also be able to do:

#[tokio::run]
async fn run_server(port: usize) -> Result<(), Error> {
    unimplemented!()
}

fn main() {
    blocking_initialization();
    if let Err(e) = run_server(1234) {
        error!(":-( {}", e);
        std::process::exit(1);
    }
    blocking_shutdown();
}

@ipetkov
Copy link
Member

ipetkov commented Sep 18, 2019

I wonder if mixing async fn definitions with #[tokio::run] will make things confusing for new comers. Normally async fn means a future is returned, but combining the two actually means "it looks like async fn on the inside, but is actually a blocking function when called from the outside".

Putting it on main doesn't matter too much since by default no-one calls it, but the example above gave me pause for a second.

@LucioFranco
Copy link
Member

I wonder if it makes sense to keep tokio::main but introduce a tokio::runtime::block_on so that your example now looks like:

async fn run_server(port: usize) -> Result<(), Error> {
    unimplemented!()
}

fn main() {
    blocking_initialization();
    if let Err(e) = tokio::runtime::block_on(run_server(1234)) {
        error!(":-( {}", e);
        std::process::exit(1);
    }
    blocking_shutdown();
}

This seems like it gets us closer to your goal while still keeping main to make getting started easier?

@vorner
Copy link
Contributor Author

vorner commented Sep 19, 2019

To be honest, I was actually OK with creating a Runtime manually and calling block_on on it and after a bit searching through the docs, it is fairly easy to do. But again, I'm not exactly a beginner so I had an idea what I was looking for.

Renaming the attribute, so it hints it can be applied to other things than just main was suggested in the gitter channel and as I liked the idea, I created the issue ‒ with the addition that if it's supposed to be actually useful in my use case, it should also accept parameters.

But maybe it's OK to just leave it as it is but have one of the well visible examples use some other mechanism than the attribute? So beginners know there's a choice and roughly in which direction to search the documentation.

@LucioFranco
Copy link
Member

I think we could also do a better job in the docs at explaining the runtime. So maybe the improvements need to go there as well?

@DoumanAsh
Copy link
Contributor

tokio::entry is something that I want myself as name too

I also will lift restriction on arguments for non-main functions

@carllerche
Copy link
Member

If we change it, my two votes would be #[tokio::enter] and #[tokio::start].

That said, the tokio::main macro is intended to be very light sugar to start a tokio process. Starting a runtime by hand isn't terribly hard. One option would be to keep #[tokio::main] for main functions but include in the documentation of the attr macro a reference to Runtime::new.

@tekjar
Copy link
Contributor

tekjar commented Sep 27, 2019

Would it also be possible to do

#[tokio::enter(single_thread)]
thread::spawn(move || {

})

@vorner
Copy link
Contributor Author

vorner commented Sep 27, 2019

@tekjar Honestly, that code feels confusing to me. What does it block on, on the closure or on the call to thread::spawn? Logically the former, syntactically the latter.

Maybe you wanted something like?:

thread::spawn(#[tokio::enter(single_thread)] move {

});

(personally, I don't have opinion if this is useful)

@tekjar
Copy link
Contributor

tekjar commented Sep 27, 2019

Maybe you wanted something like?:

Yeah what ever makes sense logically && syntactically.

I'm planning to return a Stream from one of the APIs and users can

thread::spawn(#[tokio::enter(single_thread)] move {
    for message in stream {} (assuming for loop support. or is this possible already?)
});

Instead of creating a function with #[tokio::enter(single_thread)]

On a different note, is it possible to support for loop streams with #[tokio::enter(single_thread)] directly. (I don't know anything about rust macros)

@DoumanAsh
Copy link
Contributor

DoumanAsh commented Oct 1, 2019

tl;dr now tokio::main should work with arguments as long as function name is not main
Do note it is just merged so you can only play with it on master.

As for macro itself I believe its functionality should remain as it is, and name can be whatever people feel more fitting.
Personally I think something like tokio::entry would be fine

We can even lift async requirement, but I don't think it is wise because it would not be so obvious why you can use await within it

@tekjar extending this macro to something more than functions seems just complexity for no reason.
Current form (i.e. accepting async function) makes things simple.
I don't mind to extend it to async blocks and other things, but I don't feel like it is beneficial

@carllerche
Copy link
Member

I don't think we're going to rename the attribute and, as mentioned, it works w/ arguments now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants