[bug] ctrl-c doesn't cleanup child process

[prabirshrestha]

Describe the bug

Child process is not killed when using ctrl-c.

using moon 1.7.2, rust 1.70.0, pnpm 8.5.1, vite 4.3.2

Steps to reproduce

1. I have react app using vite and a Cargo app.
2. app/client/moon.yml for vite app

tasks:
  dev:
    command: 'pnpm dev'
    options:
      runInCI: false
      persistent: true

3. app/server/moon.yml for cargo app. for watchexec use rust.bin watchexec-cli in .moon/toolchain.yml

tasks:
  dev:
    command: 'watchexec --restart -w apps/server "cargo run --package pkg1"'
    options:
      runFromWorkspaceRoot: true
      runInCI: false
      persistent: true

4. Run moon run :dev --log debug and wait for servers to both client and server to start.
5. Press ctrl-c.

Expected behavior

Expect both client and server to terminate but only client is terminated and server continues to run.

Logs

I do see the following error. So my hunch here is the client does get killed (which it does) but since pnpm fails with exit 1 it probably fails and doesn't continue to kill the rest of the remaining tasks that started. I don't see any logs on the server quitting when pressing ctrl-c.

client:dev |  ELIFECYCLE  Command failed with exit code 1.
▪▪▪▪ client:dev (15s 194ms, 94ea0c70)
[ERROR 21:12:14] log  Encountered a critical error, aborting the action pipeline  log.target="moon:action-pipeline:batch:3" log.module_path="moon_action_pipeline::pipeline" log.file="crates/core/action-pipeline/src/pipeline.rs" log.line=194
Error:   × Process pnpm failed with a 1 exit code.

[milesj]

@prabirshrestha Just released a patch in 1.7.3, let me know if that works.

[prabirshrestha]

Don't think it works.

Works:

tasks:
  dev:
    command: 'cargo run --package pkg1"'
    options:
      runFromWorkspaceRoot: true
      runInCI: false
      persistent: true

Doesn't work:

tasks:
  dev:
    command: 'watchexec --restart -w apps/server "cargo run --package pkg1"'
    options:
      runFromWorkspaceRoot: true
      runInCI: false
      persistent: true

Running watchexec --restart -w apps/server "cargo run --package objstor" from terminal works.

[prabirshrestha]

You can get my sample repro at https://github.com/prabirshrestha/moonrepo-rust-vite-template.

Download latest moon. Then run moon run dev and try pressing ctrl-c. The vite client app will exit but server cli will continue to run.

[milesj]

@prabirshrestha How are you verifying that the server keeps running? I'm using ps aux and it seems to work correctly.

While running:

ps aux | grep cargo
miles            55688   0.0  0.0 408626880   1312 s007  S+   12:48PM   0:00.00 grep --color=auto cargo
miles            55570   0.0  0.1 408949648  11360 s003  S+   12:48PM   0:00.03 watchexec -r -w crates cargo run --package cli

After ctrl+c:
While running:

ps aux | grep cargo
miles            55688   0.0  0.0 408626880   1312 s007  S+   12:48PM   0:00.00 grep --color=auto cargo

[prabirshrestha]

I'm using the same command as you are doing including my app telling I can't connect to the database (surrealdb via rocksdb) since it is in use.

Here is a minimal repo project that you can try. Tried it with v1.10.0.

1. Download and run the project.

git clone https://github.com/prabirshrestha/moonrepo-rust-vite-template
cd moonrepo-rust-vite-template
moon run dev

2. Open http://localhost:8080. It should open.
3. Press ctrl+c to quit moon run dev. Notice that it stop.
4. Open http://localhost:8080 and it should still be accessible since, the process never got terminate.

Not sure if this is due to salvo web framework or moon. I'm also running this on archlinux so not sure if it is specific to linux.

[prabirshrestha]

probably one way to solve this would be to create process group such that one can easily kill all jobs on the same group.

watchexec support sending custom signals and timeouts so we can gracefully shutdown the commands. That is something that could be supported.

 command: systemfd --no-pid -s http::8080 -- watchexec -r --stop-signal SIGTERM --stop-timeout=5 -w crates "cargo run"

[milesj]

Yeah good idea. I'll look into a better way to handle signals, right now it's not my favorite implementation :P