[bug] `moon run` doesn't wait for process to exit on Ctrl-C

[JeremyMoeglich]

Describe the bug

I have a persistent task which runs a program, this program has some exit logic that runs after tokio::signal::ctrl_c().await.unwrap(); or in general after ctrl-c When I run it directly this logic runs, if I run via moon run it sometimes executes a little, but is always stopped before finishing

Steps to reproduce

fn main() {
    ctrlc::set_handler(move || {
        std::thread::sleep(std::time::Duration::from_secs(1));

        println!("This never gets printed");
        std::process::exit(0);
    })
    .expect("Error setting Ctrl-C handler");

    loop {
        std::thread::sleep(std::time::Duration::from_secs(1));
    }
}

testtask:
    command: "cargo run"
    description: "Test task"
    options:
      persistent: true

Exits immediately

Expected behavior

Should wait before exiting like it does when running directly

Environment

System: OS: Windows 11 10.0.22631 CPU: (20) x64 13th Gen Intel(R) Core(TM) i5-13500T Memory: 5.18 GB / 31.70 GB Binaries: Node: 20.15.1 - ~\AppData\Local\fnm_multishells\120760_1728461189958\node.EXE npm: 10.7.0 - ~\AppData\Local\fnm_multishells\120760_1728461189958\npm.CMD bun: 1.1.29 - ~.proto\shims\bun.EXE Managers: Cargo: 1.80.1 - ~.cargo\bin\cargo.EXE pip3: 24.1.2 - ~.proto\shims\pip3.EXE Utilities: Git: 2.44.0. - C:\Program Files\Git\cmd\git.EXE Clang: 19.1.0 - C:\Program Files\LLVM\bin\clang.EXE Curl: 8.7.1 - C:\Windows\system32\curl.EXE Virtualization: Docker: 27.2.0 - C:\Program Files\Docker\Docker\resources\bin\docker.EXE Docker Compose: 2.28.1 - C:\Program Files\Docker\Docker\resources\bin\docker-compose.EXE IDEs: VSCode: 0.41.3 - c:\Users\moeglich\AppData\Local\Programs\cursor\resources\app\bin\code.CMD Languages: Java: 21.0.4 - C:\Program Files\Eclipse Adoptium\jdk-21.0.4.7-hotspot\bin\javac.EXE Python: 3.12.5 - C:\Users\moeglich.proto\shims\python.EXE Python3: 3.12.5 - C:\Users\moeglich.proto\shims\python3.EXE Rust: 1.80.1 - C:\Users\moeglich.cargo\bin\rustc.EXE Databases: PostgreSQL: 16.3 - C:\Program Files\PostgreSQL\16\bin\postgres.EXE Browsers: Edge: Chromium (128.0.2739.79) Internet Explorer: 11.0.22621.3527

Additional context

I'll test this on linux later, right now I've only tested windows

[JeremyMoeglich]

Also applies to linux

[milesj]

We abort threads in tokio: https://docs.rs/tokio/latest/tokio/task/index.html#cancellation

I'm pretty sure this just kills the thread and doesn't actually pass SIGINT through.

[JeremyMoeglich]

I assume it would be possible to attach a different behaviour on drop here

https://github.com/moonrepo/moon/blob/c6ea612696bb8fd93cc600ff718817d65808198d/crates/process/src/command.rs#L97-L100

I'll look into this a little more. The main issue may be that drop is sync and you shouldn't block the runtime

[JeremyMoeglich]

https://github.com/tokio-rs/tokio/issues/2504 talks about this issue. Also proposes some solutions, though in this case the correct solution may require awaiting exit rather than assuming drop to do that for you

[JeremyMoeglich]

Trying to fix this myself.

The behaviour I am going for is it waiting for all processes to exit explicitly via SIGTERM. While keeping the SIGKILL on drop in case of panic unwind or any other unexpected drop.

This will lead to it being stuck if the process doesn't respect SIGTERM. Adding a timeout of a random duration doesn't feel right either though. I'll worry about that later

[JeremyMoeglich]

This will take a while, there are no crates that do this in a way that would work on windows and unix. Even cargo run does not handle this properly since it only redirects Ctrl-C on windows not the other handlers.

I think I'll move my current logic into external crates since it's too generic to fit here

[milesj]

Yeah, I've noticed the rust ecosystem doesn't have really good support for cross-platform signals, or ways to achieve this easily.