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

rt: add Runtime::shutdown_timeout #2186

Merged
merged 7 commits into from
Jan 29, 2020
Merged

rt: add Runtime::shutdown_timeout #2186

merged 7 commits into from
Jan 29, 2020

Conversation

carllerche
Copy link
Member

Provides an API for forcing a runtime to shutdown even if there are
still running tasks.

Fixes #1923

Provides an API for forcing a runtime to shutdown even if there are
still running tasks.
@carllerche carllerche requested a review from hawkw January 28, 2020 06:32
///
/// This currently is never returned, but might at some point in the future.
#[derive(Debug)]
pub(crate) struct ParkError {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reshuffling internal code. This used to be public, which is why ParkError is opaque.

let mut shared = self.spawner.inner.shared.lock().unwrap();

if shared.shutdown {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function can be called multiple times. First, by explicitly calling shutdown then by the drop handler calling shutdown. This prevents shutting down twice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good if there was a comment in the code stating that, as well as on the PR?

@hawkw
Copy link
Member

hawkw commented Jan 28, 2020

@carllerche looks like rustfmt still needs to be run?

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me — i commented on a few minor style nits & a little docs proofreading, but the change seems good!

tokio/src/runtime/blocking/mod.rs Show resolved Hide resolved
let mut shared = self.spawner.inner.shared.lock().unwrap();

if shared.shutdown {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good if there was a comment in the code stating that, as well as on the PR?

tokio/src/runtime/blocking/shutdown.rs Show resolved Hide resolved
tokio/src/runtime/enter.rs Outdated Show resolved Hide resolved
Comment on lines +115 to +118
use crate::park::{CachedParkThread, Park};
use std::pin::Pin;
use std::task::Context;
use std::task::Poll::Ready;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor style nit: some of these imports are now used by both block_on and block_on_timeout, should they be moved out of the individual functions?

tokio/src/runtime/mod.rs Outdated Show resolved Hide resolved
tokio/src/runtime/mod.rs Outdated Show resolved Hide resolved
tokio/src/runtime/mod.rs Outdated Show resolved Hide resolved
carllerche and others added 4 commits January 29, 2020 11:06
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
@carllerche carllerche merged commit 9d6b994 into master Jan 29, 2020
@carllerche carllerche deleted the shutdown-timeout branch February 1, 2020 17:58
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

Successfully merging this pull request may close these issues.

Runtime::Drop waits for blocking synchronous code
2 participants