denoland / deno

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

Error aborting a child process' signal right when it exits #27112

Open 0f-0b opened 6 days ago

0f-0b commented 6 days ago
const controller = new AbortController();
Deno.addSignalListener("SIGCHLD", () => controller.abort());
await new Deno.Command("true", { signal: controller.signal }).output();

On a Mac running macOS 15.1, the abort call sometimes throws a TypeError.

$ deno run --allow-run=true a.js
$ deno run --allow-run=true a.js
error: Uncaught (in promise) TypeError: Child process has already terminated.
Deno.addSignalListener("SIGCHLD", () => controller.abort());
                                                   ^
    at ChildProcess.kill (ext:runtime/40_process.js:357:5)
    at onAbort (ext:runtime/40_process.js:299:32)
    at AbortSignal.[[[runAbortSteps]]] (ext:deno_web/03_abort_signal.js:182:9)
    at AbortSignal.[[[signalAbort]]] (ext:deno_web/03_abort_signal.js:166:24)
    at AbortController.abort (ext:deno_web/03_abort_signal.js:304:30)
    at …/a.js:2:52
    at loop (ext:runtime/40_signals.js:77:7)
    at eventLoopTick (ext:core/01_core.js:175:7)
$ deno --version
deno 2.1.1+1e51b65 (canary, release, aarch64-apple-darwin)
v8 13.0.245.12-rusty
typescript 5.6.2

I wasn't able to reproduce this issue in a single-core Linux VM.

lucacasonato commented 6 days ago

This behaviour occurs because while killing of child processes is an async operation, the triggering of the killing is a sync operation and the child may have died already from the SIGCHLD (or a natural exit) before you try to call abort().

This is a bug in Deno. At the minimum we should ensure that the call to childProcess.kill() inside of the onAbort handling of ChildProcess has a try {} catch {} around it that swallows the error. Secondly we should also consider suppressing the error altogether from childProcess.kill() when the child is already dead.