Skip to content

Allow running Task::local from any thread#199

Closed
oblique wants to merge 1 commit intosmol-rs:masterfrom
oblique:improve-task-local
Closed

Allow running Task::local from any thread#199
oblique wants to merge 1 commit intosmol-rs:masterfrom
oblique:improve-task-local

Conversation

@oblique
Copy link

@oblique oblique commented Jul 25, 2020

This enables us to use Task::local from any thread, this is how smol 0.1 behavior was.
We achieve this by having a thread local LocalExecutor.

The following is now possible:

use smol::Task;

fn main() {
    smol::run(async {
        Task::spawn(async {
            Task::local(async {
                println!("{:?}", std::thread::current().id());
            })
            .await;
        })
        .await;
    })
}

This commit needs smol-rs/async-executor#4, so CI will fail.

@ghost
Copy link

ghost commented Jul 25, 2020

What I like about the current behavior is that smol::run() is fully self-contained and does cleanup at the end of its invocation. That means no tasks will be "leaked" for the next invocation of smol::run() or until the thread exits.

Do you have a particular use case for the behavior proposed in this PR?

@oblique oblique force-pushed the improve-task-local branch from 105d01e to 6194d6f Compare July 25, 2020 17:07
@oblique
Copy link
Author

oblique commented Jul 25, 2020

What I like about the current behavior is that smol::run() is fully self-contained and does cleanup at the end of its invocation. That means no tasks will be "leaked" for the next invocation of smol::run() or until the thread exits.

I didn't thought of that. I edited my commit to not use thread_local. Now I create LocalExecutor within closures.

Do you have a particular use case for the behavior proposed in this PR?

Yes, and it is very difficult to guarantee that Task::local is called on the main thread.

@ghost
Copy link

ghost commented Aug 28, 2020

I think this is now a stale PR :) Thanks for the work on it, though!

@ghost ghost closed this Aug 28, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant