Skip to content

Task panic should default to aborting process #19610

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
MichaelGG opened this issue Dec 7, 2014 · 6 comments
Closed

Task panic should default to aborting process #19610

MichaelGG opened this issue Dec 7, 2014 · 6 comments

Comments

@MichaelGG
Copy link

It seems that currently (nightly 2014-12-05), the default behaviour for a panicking task is to kill the task but leave the process unharmed. That's undesireable by default, and should only happen on an opt-in manner when spawning, or a global handler. Otherwise it's too easy to have a critical task die and the process continues incorrectly. FWIW the CLR swallowed background exceptions in the first release but changed that behavior in V2.

Repro example:

fn main() {
   spawn(proc() { 
        panic!("worker is dead"); 
    });
    println!("{}", std::io::stdio::stdin().read_line());
}
@quantheory
Copy link
Contributor

It seems that currently (nightly 2014-12-05), the default behaviour for a panicking task is to kill the task but leave the process unharmed.

This is quite deliberate, and the standard library is designed around said behavior:

http://doc.rust-lang.org/guide-tasks.html#handling-task-panics

At the least, it seems that there would need to be a RFC to change this.

That's undesireable by default, and should only happen on an opt-in manner when spawning, or a global handler. Otherwise it's too easy to have a critical task die and the process continues incorrectly.

Personally, I disagree. For programs that communicate with spawned tasks in the typical way (using std::comm), continuing after a task panics is in practice already an "opt-in" behavior, because panics spread whenever communication occurs, unless explicitly dealt with by the surviving tasks.

For the great majority of real world programs, it seems reasonable to assume that spawned tasks will communicate in some way with the main task (or with a parent that ultimately communicates with the main task). This will be even more true in the future, due to Rust switching to detached threads (see this comment on issue #19245, which is itself relevant). If the main task never checks on spawned threads, there is potentially a race condition where it could exit and the process would die before other tasks have a chance to complete. (Notable exception: if the main task loops forever it obviously will not exit first.)

@steveklabnik
Copy link
Member

Yes, this is the intended current behavior. As @quantheory mentions, this would require an RFC to change, and we already have an issue for it open: rust-lang/rfcs#471

@edef1c
Copy link
Contributor

edef1c commented Dec 7, 2014

@steveklabnik That's a different issue — if there is an error handler for a task, all is well. If there's no top-level error handler for a task and it panics, that's an abort.

@steveklabnik
Copy link
Member

Regardless, this kind of issue belongs in the RFC repo.

@edef1c
Copy link
Contributor

edef1c commented Dec 8, 2014

Accepted. I hadn't considered poisoning yet, so that does decrease the amount I care about this significantly, perhaps even to zero.

@MichaelGG
Copy link
Author

Consider the common pattern of a background thread that refreshes data, say, by reloading a cache. Or a fire-and-forget task one might write, like sending an email. Rust's current behaviour allows those to just fail silently. Silent failure seems contrary to everything else Rust does. Is there a way to set or unset an error handler for tasks, or is this an all-or-nothing thing?

RFC 471 is about changing panic! to abort! which is totally different. This is simply about "don't default to silently failing". I'll open an RFC.

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

No branches or pull requests

4 participants