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.

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

Zombie subprocesses #7087

Closed
asantos00 opened this issue Aug 17, 2020 · 11 comments
Closed

Zombie subprocesses #7087

asantos00 opened this issue Aug 17, 2020 · 11 comments
Labels
bug Something isn't working correctly cli related to cli/ dir

Comments

@asantos00
Copy link

Currently, when creating a subprocess using Deno.run, the process stays in "zombie mode" even after it is killed.

I've created a gist that reproduces this bug, and talked with @lucacasonato that confirmed it.

https://gist.github.com/asantos00/bdd4357cb91f18b368085eb44105ea97

@caspervonb
Copy link
Contributor

Example from the gist

const command = ['deno', 'run', './script.ts'];

const p1 = Deno.run({ cmd: command })
const p2 = Deno.run({ cmd: command })
const p3 = Deno.run({ cmd: command })
const p4 = Deno.run({ cmd: command })

p1.kill(Deno.Signal.SIGKILL)
p2.kill(Deno.Signal.SIGKILL)
p3.kill(Deno.Signal.SIGKILL)
p4.kill(Deno.Signal.SIGKILL)

setInterval(() => {
  // just to keep it running so you can `ps aux | grep deno` to check the zombie processes
}, 1000);

@denoland denoland locked and limited conversation to collaborators Aug 28, 2020
@denoland denoland unlocked this conversation Aug 28, 2020
@lucacasonato lucacasonato added bug Something isn't working correctly cli related to cli/ dir labels Aug 28, 2020
@bartlomieju bartlomieju mentioned this issue Oct 10, 2020
22 tasks
@bnoordhuis
Copy link
Contributor

For background: UNIX expects you to reap zombies with waitpid(2) when you receive a SIGCHLD signal.

This would normally be an easy fix - set the SIGCHLD signal handler to SIG_IGN and let the kernel handle it - except....

tokio::process::Child installs its own SIGCHLD handler that records events into a queue. There is nothing polling the command objects and so events stay in the queue and zombies aren't reaped.

A crude hack that papers over the issue:

diff --git a/cli/ops/process.rs b/cli/ops/process.rs
index 60a6d5095..b259d9531 100644
--- a/cli/ops/process.rs
+++ b/cli/ops/process.rs
@@ -114,6 +114,13 @@ fn op_run(
 
   // Spawn the command.
   let mut child = c.spawn()?;
+
+  #[cfg(unix)]
+  unsafe { libc::signal(libc::SIGCHLD, libc::SIG_IGN); } // Reset signal handler.
+
   let pid = child.id();
 
   let stdin_rid = match child.stdin.take() {

It's not a good fix however because of the race window between the spawn and signal calls.

So far I haven't been able to come up with a better idea than poll child objects in the resource table at a fixed interval.

@Soremwar
Copy link
Contributor

I can reproduce this in latest canary. Linux reports all the child processes as zombies

@nktpro
Copy link

nktpro commented Jan 21, 2021

This is definitely still an issue in latest deno release 1.7.0 and it's a big problem for apps that constantly spawn a lot of child processes. We did end up with non-responsive servers after a huge amount of zombie processes were leaked.

Is there any workaround we can do temporarily for now at the application layer before it's fixed in deno runtime, other than letting the deno parent process dies when any child process is killed (which is of course not ideal for long running servers)?

@muzuiget
Copy link

muzuiget commented Feb 1, 2021

This issue still happens on v1.7.1

@satyarohith
Copy link
Member

This still seems to be an issue on 1.9.2.

@ebebbington
Copy link
Contributor

Happens when i run firefox as a subprocess on windows

@ebebbington
Copy link
Contributor

Any hope on getting this addressed?

@GJZwiers
Copy link
Contributor

As an alternative, this issue does not appear to happen when using the Deno.Command API.

@Hajime-san
Copy link
Contributor

We should migrate to Deno.Command API because of will be soft-removed in Deno 2.0.
https://deno.land/[email protected]?s=Deno.run

@littledivy
Copy link
Member

I'll close this issue because Deno.run is deprecated and will be removed in 2.0.

Upgrade to Deno.Command API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly cli related to cli/ dir
Projects
None yet
Development

No branches or pull requests