panic! in Command child forked from non-main thread results in exit status 0

[ijackson]

If a panic occurs in a child process forked off by Command on a thread other than the main thread, the whole child process exits 0, so it looks like things have worked when in fact they've gone catastrophically wrong. This seems like a serious error handling bug.

@rustbot modify labels +T-libs +libs-impl

Steps to reproduce

use std::io;
use std::process::Command;
use std::os::unix::process::CommandExt;
use std::thread;

fn main() -> Result<(),io::Error> {
    let thr = thread::spawn(||{
        let mut c = Command::new("echo");
        c.arg("hi");
        unsafe {
            c.pre_exec(|| panic!("crash now!"));
        }
        let st = c.status().expect("get status");
        println!("{:?}", st);
    });
    thr.join().expect("joined");
    Ok(())
}

Actual behaviour

thread '<unnamed>' panicked at 'crash now!', t.rs:11:27
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
ExitStatus(ExitStatus(0))

Expected behaviour

• The spawned thread builds the Command.
• Command.status tries to run the command, so
• Command.status forks, and then runs the pre_exec closure
• The pre_exec closure panics
• Consequently, the child process exits with SIGABRT or in any case nonzero
• st is not a success

Specifically, the last line should be something like ExitStatus(ExitStatus(25856)) (which is what you get without the thread::spawn)

Meta

rustc --version --verbose:

rustc 1.50.0-nightly (3ff10e74a 2020-12-04)
binary: rustc
commit-hash: 3ff10e74a74ed093fcabac1de27fe1cd65bbbb4a
commit-date: 2020-12-04
host: x86_64-unknown-linux-gnu
release: 1.50.0-nightly

Backtrace (not particularly enlightening)

``` thread '' panicked at 'crash now!', t.rs:11:27 stack backtrace: 0: std::panicking::begin_panic 1: t::main::{{closure}}::{{closure}} 2: as core::ops::function::FnMut>::call_mut at /rustc/1773f60ea5d42e86b8fdf78d2fc5221ead222bc1/library/alloc/src/boxed.rs:1314:9 3: std::sys::unix::process::process_inner::::do_exec at /rustc/1773f60ea5d42e86b8fdf78d2fc5221ead222bc1/library/std/src/sys/unix/process/process_unix.rs:228:13 4: std::sys::unix::process::process_inner::::spawn at /rustc/1773f60ea5d42e86b8fdf78d2fc5221ead222bc1/library/std/src/sys/unix/process/process_unix.rs:58:36 5: std::process::Command::status at /rustc/1773f60ea5d42e86b8fdf78d2fc5221ead222bc1/library/std/src/process.rs:904:9 6: t::main::{{closure}} note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. ExitStatus(ExitStatus(0))

</p>
</details>

[rustbot]

Error: Label libs-impl can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

[ijackson]

@rustbot modify labels +T-libs

[jonas-schievink]

Unwinding from a pre_exec callback is probably UB

[ijackson]

Jonas Schievink writes ("Re: [rust-lang/rust] panic! in `Command` child forked from non-main thread results in exit status 0 (#79740)"):

Unwinding from a pre_exec callback is probably UB

This isn't documented AFAICT. There is an extensive discussion of the strange state in the pre_exec callback but nothing saying one shouldn't panic. Furthermore, presumably any panicking bugs in Command itself (or anything it calls) will have the same effect. Perhaps the same thing could occur in a Rust library being used in a mostly-C program which made a thread and then forked. Ian.

…

-- Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own. Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk, that is a private address which bypasses my fierce spamfilter.

[tmiasko]

Trying to catch the unwind seems plausible even if attempting to unwind causes undefined behaviour already. Alternatively the callbacks could be invoked through a function with #[unwind(aborts)] attribute.

[ijackson]

Here is another program with weird behaviour:

use std::process::Command;
use std::os::unix::process::CommandExt;
use std::panic::catch_unwind;

fn main() {
    let got = catch_unwind(||{
        let mut c = Command::new("echo");
        c.arg("hi");
        unsafe {
            c.pre_exec(|| panic!("crash now!"));
        }
        let st = c.status().expect("get status");
        println!("{:?}", st);
    });
    eprintln!("got={:?}", &got);
}

Output

thread 'main' panicked at 'crash now!', src/main.rs:10:27
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
got=Err(Any)
got=Ok(())

Demonstrating that catch_unwind returned twice!

I think the solution has to be that in the child process, unwinding must stop at fork. I will send an MR.