Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
0f-0b opened this issue Nov 27, 2024 · 1 comment · May be fixed by #27124 or #27641
Open

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

0f-0b opened this issue Nov 27, 2024 · 1 comment · May be fixed by #27124 or #27641
Labels
bug Something isn't working correctly good first issue Good for newcomers runtime Relates to code in the runtime crate

Comments

@0f-0b
Copy link
Contributor

0f-0b commented Nov 27, 2024 

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
Copy link
Member

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.

@lucacasonato lucacasonato added bug Something isn't working correctly good first issue Good for newcomers runtime Relates to code in the runtime crate labels Nov 27, 2024
@yuxi-ovo yuxi-ovo linked a pull request Nov 28, 2024 that will close this issue
Open
@MohammadSu1 MohammadSu1 linked a pull request Jan 12, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working correctly good first issue Good for newcomers runtime Relates to code in the runtime crate
Projects
None yet
Milestone
No milestone
2 participants
@lucacasonato @0f-0b