Skip to content

Commit

Permalink
Make zombie children clean themselves up
Browse files Browse the repository at this point in the history
The parent wds process can crash sometimes when watching too many files, or due to memory pressure on the system, or really any number of reasons. When this happens, the children become zombies and live forever. If the parent crashes though, they'll never be reloaded, and signals from the terminal won't be sent along to them, so they just accumulate and suck up memory on developer machines! Yuck!

This adds active monitoring to the child wds processes so that they notice if the parent is gone, and kill themselves right away if so. This isn't 100% reliable, but this will at least reap most of em so we stop accumulating so much cruft.
  • Loading branch information
airhorns committed Feb 11, 2024
1 parent 8f60ee6 commit 46309fa
Show file tree
Hide file tree
Showing 27 changed files with 334 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
- uses: actions/checkout@v2
- uses: ./.github/actions/setup-test-env
- run: pnpm build
- run: test/test.sh
- run: integration-test/test.sh
- run: pnpm jest

lint:
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
10 changes: 10 additions & 0 deletions integration-test/parent-crash/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "parent-crash",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"license": "ISC"
}
4 changes: 4 additions & 0 deletions integration-test/parent-crash/run.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
process.stderr.write("child started\n")
setInterval(() => {
process.stderr.write("child still alive\n")
}, 200)
50 changes: 50 additions & 0 deletions integration-test/parent-crash/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env zx
const assert = require("assert");
const { exec } = require("child_process");

async function getChildPids(parentPid, callback) {
const result = await $`pgrep -P ${parentPid}`;
return result.stdout
.split("\n")
.filter((pid) => pid)
.map((pid) => parseInt(pid, 10));
}

function processIsRunning(pid) {
try {
process.kill(pid, 0);
return true;
} catch (e) {
return false;
}
}

const { setTimeout } = require("timers/promises");

const main = async () => {
// launch the wds process
const parent = $`${__dirname}/../../pkg/wds.bin.js --watch ${__dirname}/run.ts`.nothrow();

// wait for the wds process to start
await setTimeout(500);

// get the pid of the child process that the parent wds supervisor will have started
const pids = await getChildPids(parent.child.pid);
assert(pids.length > 0, "no child pids found for supervisor process");

// SIGKILL the parent process, as if it OOMed or something like that to simulate a zombie child
console.log(`killing parent (${parent.child.pid})`);
await parent.kill(9);
assert.ok(processIsRunning(pids[0]), "test is broken, child process is not running immediately after parent is dead");

// ensure the children are dead too after their monitoring delay
await setTimeout(3000);

for (const pid of pids) {
assert.ok(!processIsRunning(pid), `child process ${pid} is still running after parent has been killed`);
}

await parent;
};

void main();
File renamed without changes.
10 changes: 10 additions & 0 deletions integration-test/reload/run-scratch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const http = require("http");

const requestListener = function (req, res) {
res.writeHead(200);
res.end("Hey, Pluto!");
};

const server = http.createServer(requestListener);
server.listen(8080);
console.warn("Listening on 8080");
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
5 changes: 5 additions & 0 deletions test/test.sh → integration-test/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,9 @@ echo
echo "::group::Reload test ${args}"
bash $DIR/reload/test.sh $args
echo "::endgroup::"
echo

echo "::group::ParentCrash test ${args}"
node_modules/.bin/zx $DIR/parent-crash/test.js $args
echo "::endgroup::"
echo
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ module.exports = {
"^.+\\.[jt]sx?$": ["@swc/jest", { sourceMaps: "inline" }],
},
testEnvironment: "node",
testMatch: ["<rootDir>/spec/**/?(*.)+(spec|test).[tj]s?(x)"],
};
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@
"gitpkg": "^1.0.0-beta.2",
"jest": "^27.4.7",
"prettier": "^2.8.8",
"typescript": "^5.1.3"
"typescript": "^5.1.3",
"zx": "^7.2.3"
},
"packageManager": "pnpm@8.12.1+sha256.28ca61ece5a496148b73fabc9afb820f9c3fec4f55f04ce45a2cea0a5219f2e1"
}

0 comments on commit 46309fa

Please sign in to comment.