Skip to content

task::block_on -> thread::spawn_task #301

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

Closed
wants to merge 4 commits into from
Closed

Conversation

yoshuawuyts
Copy link
Contributor

Replaces task::block_on with thread::spawn_task. Ref #289 #299. Not sure if this is a great idea or terrible, but figured I'd open it anyway. We should probably think about this long and hard before we decide to add this.

This also means the addition of a new thread submodule.

Oh another gotcha is that because we want to block the current thread, we can't get back a JoinHandle. We could jump through hoops to spawn a new thread and get a handle back that way -- but that seems like a somewhat odd choice to make, and I think we should not spawn new threads just for this purpose.

Thanks!

cc/ @stjepang

Signed-off-by: Yoshua Wuyts <[email protected]>
Signed-off-by: Yoshua Wuyts <[email protected]>
Signed-off-by: Yoshua Wuyts <[email protected]>
@bill-myers
Copy link

"spawn" is associated with starting something without waiting for it, but this waits for it.

I think thread::run_task, thread::execute_task or even just thread::execute are better names.

@yoshuawuyts yoshuawuyts added the enhancement New feature or request label Oct 13, 2019
@skade
Copy link
Collaborator

skade commented Oct 28, 2019

How about task::main, task::block_thread? Other then that, I like thread::execute_task.

I think we should keep task as the main module to use, people will orient towards it for all task spawning needs.

We could have an alias here.

@yoshuawuyts
Copy link
Contributor Author

@skade I think those are both reasonable suggestions. Either way, we shouldn't merge this PR as-is right now so I'm going to go ahead and close it. Let's ponder on this for a bit.

@yoshuawuyts yoshuawuyts deleted the thread-spawn-task branch October 28, 2019 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants