Shim executables don't ensure termination of child processes

[mitranim]

Duplicating from chocolatey/choco#2272. Both package managers share this problem.

What You Are Seeing?

Abrupt termination of a shim executable causes its child to be orphaned. The child becomes a daemon process.

Note that this doesn't apply to Ctrl+C from the CLI, which is treated differently by the terminal or OS. The problem occurs when the process is killed through any other method.

What is Expected?

When a shim executable terminates for any reason, all work should be stopped, i.e. the subprocess should terminate. It should either:

• Avoid spawning a subprocess.
• Use the job objects API, ensuring that when it terminates, its descendants terminate.

How Did You Get This To Happen? (Steps to Reproduce)

This may happen due to programmatic termination of a shim process, or simply by clicking "End Process" in Task Manager or Process Explorer. Programmatic termination may happen in a build system that involves shelling out to various build tools such as sass. An error in one of the tasks may cause termination of the entire build, which may involve programmatic termination of immediate child processes, causing the orphan problem described above.

Programmatic termination is particularly important for watching-and-restarting, which is common in build systems. Currently, it's not viable to programmatically auto-restart executables generated by Scoop, because the "real" process doesn't die.

[rasa]

kiennq's shim uses jobs. See https://github.com/kiennq/scoop-better-shimexe/blob/f334a2ca21d231c0b244bb9b14b219355e60a616/shim.cpp#L212
To switch to his shim, see #3634 (comment)

[mitranim]

Thanks for the pointers. This is also a duplicate of #3294 which I failed to find before opening the issue.

Testing this with watchexec. 71's version worked when invoked manually from the terminal, but when called indirectly via make, was failing with Error -1073740940. kiennq's version seems to work in both environments, and terminates the child process properly. Issue seems to be solved. Thanks! Perhaps it should be the default, but that's a beaten horse already. 🐴🙂