nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.64k stars 29.61k forks source link

Unrefed child_process inside a worker thread becomes a zombie #46569

Open heypiotr opened 1 year ago

heypiotr commented 1 year ago

Version

v16.19.0, v18.14.0

Platform

Linux PC 5.15.79.1-microsoft-standard-WSL2 #1 SMP Wed Nov 23 01:01:46 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Darwin Piotrs-Framer-MBP.local 22.3.0 Darwin Kernel Version 22.3.0: Thu Jan  5 20:53:49 PST 2023; root:xnu-8792.81.2~2/RELEASE_X86_64 x86_64

Subsystem

child_process, worker_threads

What steps will reproduce the bug?

If you spawn a child process inside a worker thread, and then unref the child process, the process becomes a zombie when the worker thread exits.

This is a problem if you have a long-running main process that over the course of its lifetime spawns a lot of worker threads, because at some point you run out of PIDs.

index.mjs:

import { Worker } from "worker_threads";

const w = new Worker("./worker.mjs");
w.on("exit", () => console.log("worker exited"));

// keep the main process running
setTimeout(() => {}, 3600_000);

worker.mjs:

import { spawn } from "child_process";

const child = spawn("cat", { stdio: ["pipe", "pipe", "inherit"] });
child.on("spawn", () => {
  console.log(child.pid);
  child.unref();
  child.stdin.unref();
  child.stdout.unref();
});

How often does it reproduce? Is there a required condition?

Repro'es every time with the provided snippets.

What is the expected behavior?

The child process gets removed from the processes table.

What do you see instead?

$ node index.mjs
15045
worker exited

in another terminal window:

$ ps aux|grep Z
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
heypiotr 15045  0.0  0.0      0     0 pts/5    Z+   14:08   0:00 [cat] <defunct>

Additional information

No response

bnoordhuis commented 1 year ago

I suppose that isn't too surprising. Node closes libuv's uv_process_t handles when the thread exits, meaning libuv won't reap the child processes when they exit. I don't think there's a nice, clean way to fix that.

Keeping the thread and handles around until the last child process exits works but turns into a resource leak when child processes don't exit.

theanarkh commented 1 year ago

Is it recommended to create a process in a worker thread in Node.js ?

bnoordhuis commented 1 year ago

I'd say users can reasonably expect that to work. This is just a design flaw-y edge case.