[cargo run] Should proxy signals to child process

[passcod]

From watchexec/cargo-watch#25, around this comment.
Also referring this short IRC conversation.

Background: Cargo Watch has improved behaviour when watching a Cargo project and running cargo run: on changes, it kills the child cargo run process and starts a new one. This is highly helpful when developing web servers and other applications which never return. For prior art, see nodemon.

Cargo Watch currently uses the stdlib's Child#kill() method on Windows, and libc::kill() on all other platforms (Unix), with a signal of 15 (SIGTERM).

Problem: cargo run creates a new child process. Killing the cargo run child does not kill the grand-child. Cargo Watch then proceeds to start a new cargo-run process, which attempts to start a new grand-child. We now have two of the target process, which is obviously unexpected and wrong. In the case of webservers, the first target process is still bound to its TCP port, preventing latter target processes from functioning, producing EADDRINUSE errors.

Proposal: To have cargo run proxy signals to its own child process, allowing Cargo Watch (and other applications opening cargo child commands) to send a SIGTERM (on Unix) or taskkill (on Windows, through the stdlib's implementation) directly to the target process. The target would then be responsible for its own shutdown.

Cargo Watch would not attempt to send further signals, instead waiting for the cargo run process to return normally.

cargo-run may want to SIGKILL its child if it does not seem to be responding to SIGTERM, but I will leave that up to Cargo maintainers.

I am also not familiar with the behaviours on Windows, so cannot comment on that aspect.

[LukasKalbertodt]

/cc me

[kamalmarhubi]

For Windows you'll probably want to use Job Objects, and you can see an example here for how to use them. The basic idea is that a bunch of processes are added to the job object and then when the object itself is closed it'll kill every process associated with it.

―@alexchriton in watchexec/cargo-watch#25 (comment)

[passcod]

I was wondering how programs hook into their shutdown on Windows. On Unix, the polite thing to do is to SIGTERM (or SIGINT) processes, because SIGTERM is trappable so you can do stuff like closing sockets, releasing ports, and saying goodbye instead of just ceasing to exist. Does Windows have similar capabilities?

[alexcrichton]

Thanks for the report @passcod! Here's some thoughts of mine:

• Dealing with signals on unix is a huge pain and incredibly difficult to get right. If at all possible I'd prefer to never touch them!
• Along those lines, when doing a cargo run, we might arguably prefer to use exec rather than proxying signals. That'll mean that everything is "naturally proxied" and you get some other benefits like pid savings, memory savings, etc.
• Cargo spawns lots of children (e.g. rustdoc, rustc, build scripts, test harnesses, etc), and proxying signals to all of them may be a bit too cumbersome. For a tool like cargo-watch I think the best way to go on Unix is definitely using setsid to create a new process group plus killing the entire process group.
• On Windows, as far as I know, there are no equivalents of signals. The default TerminateProcess function is akin to SIGKILL. You can somewhat handle ctrl-c with SetConsoleCtrlHandler, but those seem more difficult to programmatically generated and seem simply rarer in general.
• For windows, however, job objects are pretty handy for this sort of operation of managing process groups. The JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE flag means that entire process groups can be killed all at once.

So overall, my recommendation would be something along the lines of:

• Cargo should use exec where possible (e.g. with cargo run) on Unix platforms to basically mimic "signal proxying"
• Cargo should perhaps use job objects on Windows to ensure that the immediately spawned child is killed when Cargo itself is killed. We may even want the entire process tree to be killed when Cargo is killed, but that's a little more sketchy to me.
• cargo-watch should kill the entire process group when spawning Cargo on Unix.
• cargo-watch should probably put the entire cargo invocation in its own job object to be killed all at once on Windows (recursive children as well)

Does that all make sense?

[alexcrichton]

One point about job objects on Windows which makes me a little uneasy is:

Windows 7, Windows Server 2008 R2, Windows XP with SP3, Windows Server 2008, Windows Vista, and Windows Server 2003: A process can be associated with only one job. Jobs cannot be nested. The ability to nest jobs was added in Windows 8 and Windows Server 2012.

Which to me means two things:

1. Cargo would need to be quite defensive about using job objects on Windows, e.g. resilient to failures. Before Windows 8 it may be relatively routine for Cargo itself to already be part of a job object.
2. This may be a breaking change for Cargo to start using job objects. Builds on pre-Windows 8 machines may already be using job objects for subprocesses and may not be resilient to failure, so they may not expect to already be part of a job object.

[passcod]

Is there an equivalent of exec on Windows for cargo to use on run commands? That would considerably simplify that aspect.

[passcod]

At least with respect to cargo-watch.

[alexcrichton]

If there is I'm at least unaware of such a piece of functionality which is standard. Which is to say that I don't believe it exists.

[passcod]

For Unix, cargo watch will definitely use process groups, even though it might just become irrelevant if cargo goes to use exec. At least for backward compatibility with older cargo.

For Windows, I'll have to investigate/think a bit more. I probably do not want to forgo support for Windows < 8 in cargo-watch.

[sfackler]

I've seen very weird behavior on OSX, where ctrl-cing a cargo run will sometimes kill the running process and sometimes leave it alive reparented to init. It's not totally clear to me what the conditions are that cause the behavior to change.

[alexcrichton]

Cargo doesn't itself manage child processes at all, so any weird behavior is likely just a relic of whatever the shell is doing. If a child process manually goes into a new group then when the shell kills the entire process group the child keeps going (as it's part of a different group).

[alexcrichton]

Cargo now uses job objects on Windows, so I think the only change still needed here is to use exec on Unix once it's stable.

[robinst]

CommandExt::exec is now stable in Rust 1.9, right?

[alexcrichton]

It is indeed! This can probably only be used for cargo run, but it's the "most correct" function to call in that situation.

[robinst]

I'm looking at Cargo's process_builder.rs, its ProcessBuilder has an exec method. I changed it on Unix to use CommandExt::exec and it seems to work nicely for cargo run.

But as you said, other usages should not do a exec. Would you rename the other usages (maybe spawn), or introduce a new method that does CommandExt::exec with a new name (maybe exec_replace)?

[robinst]

I've opted for the second for now, as there's more code that uses "exec" (e.g. ExecEngine). See PR #2818.

[alexcrichton]

Ah yeah adding a new exec_replace method is fine, thanks @robinst!

[Yamakaky]

Is this issue solved by the previous commits?

[alexcrichton]

Indeed!

[Yamakaky]

I imagine it will not be available until a few weeks?

[alexcrichton]

This is likely already in the nightly Cargo builds, but yeah to get to stable it'll take awhile.