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

Run basic scheduler destructor inside executor context #3966

Closed
wants to merge 2 commits into from

Conversation

Diggsey
Copy link
Contributor

@Diggsey Diggsey commented Jul 18, 2021

Fixes #3963

@Diggsey Diggsey changed the title Add a test for #3963 Fix #3963 and add a regression test Jul 18, 2021
@Darksonn Darksonn changed the title Fix #3963 and add a regression test Run basic scheduler destructor inside executor context Jul 18, 2021
@Diggsey Diggsey force-pushed the db-spawn-on-shutdown-test branch from ad90416 to 5b41b90 Compare July 18, 2021 20:50
Comment on lines -304 to 306
pub(crate) fn shutdown(mut self) {
pub(crate) fn shutdown(&mut self) {
self.spawner.shutdown();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a destructor to Runtime means we can no longer move out of the handle field, which would have made it impossible to call shutdown() on it. This was the simplest fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(See the implementation of shutdown_timeout)

@Darksonn
Copy link
Contributor

The CI failed again after restarting. I suspect there is a double panic somewhere. Try running them locally with --nocapture to see if you get any output before it aborts.

@Diggsey
Copy link
Contributor Author

Diggsey commented Jul 18, 2021

Output is as expected when I run it locally: each of the panic tests panics once.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-runtime Module: tokio/runtime labels Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-runtime Module: tokio/runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#[tokio::test] drops future outside of async runtime
2 participants