HarperFast / HarperFast/harper
waitForConfirmedTermination polls without a deadline, and processGroupIsAlive counts an unreaped zombie as alive — a spawn's cleanup can hang forever
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Summary
`waitForConfirmedTermination` polls a liveness predicate with **no deadline and no iteration bound**. When that predicate is `processGroupIsAlive`, it probes with `process.kill(-pgid, 0)` — which **succeeds for a process that has exited but not been reaped** (a zombie remains in the process table until waited on). So a spawn whose direct child ends up unreaped makes the awaiting caller poll every 25 ms forever.
Any caller awaiting `terminateProcessTree` inherits this. Observed in the field as an indefinitely hung process (details below), but the defect is in the primitive, not in one caller.
Verified against the bundled core of `@harperfast/harper-pro@5.2.0` as running in production, and the same shape is present on `main`.
## The code
`components/Application.ts`:
```ts
const PROCESS_TERMINATION_GRACE_MS = 5000;
const PROCESS_TERMINATION_POLL_MS = 25;
function processGroupIsAlive(processGroupId: number): boolean {
try {
process.kill(-processGroupId, 0);
return true;
} catch (error: any) {
return error.code === 'EPERM';
}
}
export async function waitForConfirmedTermination(
isAlive: () => boolean | Promise,
pollMs: number = PROCESS_TERMINATION_POLL_MS
): Promise {
while (await isAlive()) await delay(pollMs); // no deadline, no cap
}
```
Reached from `terminateProcessTree` on the SIGKILL escalation path:
```ts
if (!(await waitForProcessGroupExit(processGroupId, PROCESS_TERMINATION_GRACE_MS))) {
try { process.kill(-processGroupId, 'SIGKILL'); } catch (error: any) { if (error.code !== 'ESRCH') throw error; }
await waitForConfirmedTermination(() => processGroupIsAlive(processGroupId));
}
```
Note the asymmetry: the sibling `waitForProcessGroupExit(processGroupId, timeoutMs)` **is** bounded (`const deadline = performance.now() + timeoutMs; … if (performance.now() >= deadline) return false;`). Only the post-SIGKILL "confirmation" wait is unbounded.
## Why a zombie defeats it
- A process that has exited but has not been reaped still occupies a PID, so `kill(pid, 0)` / `kill(-pgid, 0)` **succeeds**. `processGroupIsAlive` therefore returns `true` for a group whose only remaining member is a zombie.
- `SIGTERM` and `SIGKILL` are no-ops against an already-dead process, so escalation cannot clear it.
- With no deadline, `waitForConfirmedTermination` cannot exit. The awaiting caller is stuck permanently.
The comment above the group probe explains the intent — "The direct child's exit does not prove the process group is empty: a custom installer can spawn-and-unref a descendant that inherits the group and outlives its parent" — which is a sound reason to probe the group. The flaw is treating an unreaped-but-dead member as indistinguishable from a live one, and then waiting on it without bound.
## Field observation
A production node hung during startup and never recovered. State after 90+ minutes, from an inspector session plus host inspection:
- exactly **one** timer, re-arming at ~16-50 ms (consistent with `PROCESS_TERMINATION_POLL_MS = 25`), and **no long timer** of any kind
- **no `process` handle** in the libuv census — the `ChildProcess` handle had already closed
- the direct spawn child (`sh`) present as a **zombie**, `PPid 1`, unreaped
- all 19 threads in state `S`; CPU ~0.76%; no log output after the first second of boot
- the two open pipes were the process's own `fd:1`/`fd:2`, not a child's
That is the signature of this loop: a 25 ms poll waiting on a zombie that can never be reaped away by signalling.
## Expected
1. **Bound `waitForConfirmedTermination`** — accept a `timeoutMs` like its bounded sibling, and on expiry either resolve with a warning or reject, rather than polling forever.
2. **Do not count an unreaped-dead member as alive.** On Linux the group member's state can be distinguished (e.g. reading `/proc//stat` state `Z`), so a group whose only remaining members are zombies should read as terminated.
3. **Log the wait.** A caller stuck here produces no output at all, which makes it undiagnosable without an inspector.
## Impact
Any code path awaiting `terminateProcessTree` can hang indefinitely and silently. The observed instance was startup — which takes the whole node out, since no listener opens until component preparation resolves — but the primitive is used for spawn cleanup generally, so the exposure is not limited to boot.
## Related
- #2072 — the field incident this was diagnosed from, and the startup-gating that makes it fatal at boot rather than merely degrading. That issue's earlier revisions attributed the hang to the component-preparation lock; that attribution is retracted there in favour of this.
- #2001, #2061, #2063 — same family: an unbounded or unsignalled wait with no health signal.
## Still unproven
Why the direct child was never reaped. `'close'` firing implies its stdio reached EOF, and Node normally reaps on `SIGCHLD`, so the zombie's persistence is not explained by this issue alone — it is the precondition that makes the unbounded poll fatal. Worth a maintainer's eye, but the unbounded wait is a defect regardless of how the zombie arises.
Filed from a field incident; cluster, host, component and repository identifiers omitted.
Contributor guide
Research direction
Start in components/Application.ts with waitForConfirmedTermination, processGroupIsAlive, and terminateProcessTree, then compare the bounded waitForProcessGroupExit implementation. Trace the SIGKILL escalation path and verify the chosen timeout, zombie handling, and diagnostic logging against the expected termination behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- backend, operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100