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

Stop runtime on task panic #2002

Open
mikhailOK opened this issue Dec 20, 2019 · 16 comments
Open

Stop runtime on task panic #2002

mikhailOK opened this issue Dec 20, 2019 · 16 comments
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-runtime Module: tokio/runtime

Comments

@mikhailOK
Copy link

Version

tokio 0.2.6

Description

I'm indirectly using tokio runtime with basic scheduler (through using actix 0.9.0).
It seems like tokio 0.1 would stop if any task panics, but 0.2.6 catches everything in task::harness::Harness::poll and the runtime keeps going.

Is there any way to get the old behavior of stopping the runtime?

@carllerche
Copy link
Member

Are you able to abort on panic? You could also set a panic_handler that signals the root task (block_on) to exit.

@mikhailOK
Copy link
Author

panic hook to signal the root task works.
Is there a plan to add API to pass a panic_handler to Harness::poll as opposed to std panic hook?

@Vlad-Shcherbina
Copy link
Contributor

Dealing with it in the panic handler is not the best option because maybe I still want to explicitly catch panics in specific scopes, but unexpected panics elsewhere should terminate the whole thing. By default. It's an unpleasant surprise when they don't (see fail-fast).

@Darksonn Darksonn added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-runtime Module: tokio/runtime labels Jul 25, 2020
@Darksonn
Copy link
Contributor

Closing in favor of #2699.

@Vlad-Shcherbina
Copy link
Contributor

It's not about tests. It's about panics being silently caught everywhere. In production too.

Anywhere else in Rust, if there is a panic in the code not explicitly wrapped in catch_unwind, the whole program terminates with a diagnostic message. This goes in line with Rust's emphasis on correctness. Panic usually indicated a bug in the code, and I don't want bugs to be silently ignored. I want bugs to be reported and fixed.

It is true that sometimes we need to catch panics to ensure robustness. For example, perhaps we don't want a panic in a request handler to terminate the whole web server program. But that's none of tokio's business! It's web framework's or even web application's business! It is possible to use tokio for something besides web applications, and in those use cases panics definitely shouldn't be silently ignored.

Consider reopening.

@Darksonn Darksonn reopened this Jul 25, 2020
@Darksonn
Copy link
Contributor

Darksonn commented Jul 25, 2020

Regarding the "anywhere else in Rust" part, I will note that we are mirroring the behavior of std::thread. See also #1830 and #1879.

@carllerche
Copy link
Member

tokio::spawn models thread::spawn. As @Darksonn mentioned, thread::spawn does not abort the process on panic. Spawned tasks are unwind-safe due to the Send + 'static bound.

In order to deviate from thread::spawn's behavior, we would need a compelling argument.

I could buy into a shutdown_on_panic flag to runtime given a compelling argument. One would have to explain why std's behavior is not sufficient (i.e. configure the process to abort on panic).

@s97712
Copy link

s97712 commented Aug 6, 2020

tokio::spawn models thread::spawn. As @Darksonn mentioned, thread::spawn does not abort the process on panic. Spawned tasks are unwind-safe due to the Send + 'static bound.

In order to deviate from thread::spawn's behavior, we would need a compelling argument.

I could buy into a shutdown_on_panic flag to runtime given a compelling argument. One would have to explain why std's behavior is not sufficient (i.e. configure the process to abort on panic).

What about spawn_local? Whether to consider to end the current thread when panic in the "local task"?

@Darksonn
Copy link
Contributor

Darksonn commented Aug 6, 2020

I don't think spawn_local and spawn should have different behaviour on this point.

@hawkw
Copy link
Member

hawkw commented Aug 6, 2020

What about spawn_local? Whether to consider to end the current thread when panic in the "local task"?

I think inconsistent behavior between spawn and spawn_local is not ideal --- it would introduce more complexity and confusion.

I could buy into a shutdown_on_panic flag to runtime given a compelling argument. One would have to explain why std's behavior is not sufficient (i.e. configure the process to abort on panic).

IMO, the main argument for a shutdown_on_panic flag is for test code. If assertions are made in code that ends up being run in a spawned task, the JoinHandles of all those spawned tasks must be awaited in the main test body to ensure panics from assertion failures are propagated. This can be unwieldy, and in some cases, it's easy to misplace a JoinHandle and forget to await it, resulting in a test that passes even if an assertion fails --- which is far from ideal. If there was a shutdown_on_panic flag, I would definitely use it in tests (and might want tokio::test to enable it).

@s97712
Copy link

s97712 commented Aug 7, 2020

If these tasks are running in the same thread and one of tasks is panic, why the other tasks still working, which makes me more confused.

@Darksonn
Copy link
Contributor

Darksonn commented Aug 7, 2020

@s97712 Panics in spawned tasks are caught, just like they are for spawned threads in std. Tasks spawned with the ordinary tokio::spawn function also share their threads in some manner.

@lamafab
Copy link

lamafab commented Dec 1, 2021

Any progress on this?

@Darksonn
Copy link
Contributor

Darksonn commented Dec 1, 2021

No, there's currently no way to do this.

@Venryx
Copy link

Venryx commented Jan 24, 2022

Having a way to tell Tokio to "not catch" panics that occur in its threads seems like a useful feature for me.

My use-case: I have my Rust program deployed in Kubernetes. When a panic occurs, I want my program to crash/completely-close, so that Kubernetes can notice the crash and perform its regular handling (eg. restarting the pod, unless it keeps crashing immediately, in which case back off for a while).

I looked through the source-code of Tokio, and could not find a way to directly achieve what I wanted. That said, here are some workarounds I have found.

Workaround 1

Enable Rust's "abort on panic" setting.

You can do this by...
A) Adding the following to your root Cargo.toml file, as seen here:

[profile.XXX]
panic = "abort"

B) Or, by adding -C panic=abort to the rustflags, as seen here.

You can control the granularity of the stack-traces logged to the console by setting the RUST_BACKTRACE environment variable:

RUST_BACKTRACE=0 # no backtraces
RUST_BACKTRACE=1 # partial backtraces
RUST_BACKTRACE=full # full backtraces

Workaround 2

Add a custom panic handler, which receives the error, prints a backtrace (optionally), and then manually aborts your program (optionally):

#![feature(backtrace)]

use std::backtrace::Backtrace;

#[tokio::main]
async fn main() {
    //panic::always_abort();
    panic::set_hook(Box::new(|info| {
        //let stacktrace = Backtrace::capture();
        let stacktrace = Backtrace::force_capture();
        println!("Got panic. @info:{}\n@stackTrace:{}", info, stacktrace);
        std::process::abort();
    }));

    [...]
}

I like this approach better because it gives me control of how much of the stacktrace to print (they can be quite long!), as well as whether the panic is of a type that is worth calling abort() for.

The one main drawback is that the backtrace-generation code (Backtrace.capture()) is currently only available on Rust nightly.


If you want to use the backtrace-generation on Rust stable, you can actually, but it requires a hack where you set this environment variable: RUSTC_BOOTSTRAP=1 (as described here)

You can set that as a global environment variable, or have it set specifically for your cargo-build command.

For Docker: Just add a ENV RUSTC_BOOTSTRAP=1 line before your build commands. (or use RUN RUSTC_BOOTSTRAP=1 <rest of command> for each command)

For rust-analyzer (in VSCode): Add this to your project's .vscode/settings.json file:

    "rust-analyzer.server.extraEnv": {"RUSTC_BOOTSTRAP": "1"}

@bouk
Copy link

bouk commented Nov 10, 2022

Related: #4516

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 C-feature-request Category: A feature request. M-runtime Module: tokio/runtime
Projects
None yet
Development

No branches or pull requests

9 participants