[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

moon/crates/process/src/command.rs

Lines 97 to 100 in c6ea612

	let mut command = TokioCommand::new(&command_line.command[0]);
	command.args(&command_line.command[1..]);
	command.envs(&self.env);
	command.kill_on_drop(true);

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]

tokio-rs/tokio#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.