denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
95.15k stars 5.28k forks source link

feat: easier way to destroy child processes #20497

Open marvinhagemeister opened 1 year ago

marvinhagemeister commented 1 year ago

After a debugging session regarding leaking resources in tests it turned out that a test case in the Fresh repository wasn't properly waiting for child processes to be killed.

The current way to do it should look like this:

const childProcess = new Deno.Command(...).spawn();

// Send kill signal
childProcess.kill("SIGTERM");
// Wait until it's shut down finished. This is often forgotten
await childProcess.status;

This lead us to wonder if we could provide an easier API for that. Especially, in a test suite it's not relevant how the child process is killed. You just want this thing to be destroyed. Some random ideas:

// New method
await childProcess.killAndWait();

// Make kill() return Promise, doesn't work with every signal though so meh
await childProcess.kill("SIGTERM")

// Add necessary symbols to support "using"
using childProcess = new Deno.Command(...).spawn();
lucacasonato commented 1 year ago

It's to be noted that with #20501 the accuracy of the op sanitizer has improved to the point where forgetting await childProcess.status will fail the test with a sanitizer error, if the sanitizers are enabled (as they are by default).

We should still ponder this, but it's less urgent now.

magurotuna commented 10 months ago

Now that delcaration with using is supported and ChildProcess implements AsyncDisposable, we can easily ensure child processes are destructed once they get out of scope:

await using proc = new Deno.Command(...).spawn();