HarperFast / HarperFast/harper
Version replacement leaves two sidecar processes running: the outgoing child's exit handler unlinks the incoming child's PID lock
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
**Location:** `security/jsLoader.ts:1117-1123` (exit handler in `createSpawn`), with `security/jsLoader.ts:1017-1032` (version-replacement branch in `acquirePidFileLock`)
## Impact
The PID file is the only thing enforcing "one process per `name` across all threads". During an `options.version` replacement the outgoing process deletes the *incoming* process's lock file, leaving a live process with no lock. The next `spawn` on any thread sees no lock and starts a **second** process. The invariant the module exists to enforce is silently broken, and nothing reports it.
## Details
The cleanup handler closes over the lock **path**, not the PID it wrote:
```js
childProcess.on('exit', () => {
try {
unlinkSync(pidFilePath); // <-- unconditional, by path
} catch { }
});
```
On a version mismatch, `acquirePidFileLock` SIGTERMs the old child, unlinks the file, busy-waits ~5ms and retries; the retry acquires the lock and `createSpawn` spawns a new child and writes *its* PID. Only then does the old child's `exit` event get delivered — the kill and respawn happen inside one synchronous run, so the handler cannot fire before the new PID file exists. It then deletes it.
## Reproduction
Driving the verbatim `createSpawn` / `acquirePidFileLock` source from `origin/main`:
```
v1 child pid=57986 pidfile="57986\n1"
v2 child pid=57995 pidfile="57995\n2" <-- correctly names the NEW child
after old child's 'exit' handler ran:
pidfile =
v2 child 57995 still alive? true
next spawn returned pid=58004 (constructor=ChildProcess)
=> processes now alive: 57995, 58004
```
Two live processes for a lock that guarantees one. Note the third call returns a real `ChildProcess`, not an `ExistingProcessWrapper` — the caller has no way to tell it just started a duplicate.
## Recommended fix
Guard the unlink on the file still naming *this* child, and make the read-compare-unlink as close to atomic as the design allows:
```js
childProcess.on('exit', () => {
try {
const { pid } = parsePidFile(readFileSync(pidFilePath, 'utf-8'));
if (pid === childProcess.pid) unlinkSync(pidFilePath);
} catch { }
});
```
This is also the reason the replacement handoff should wait for the old process to actually exit rather than busy-waiting a fixed 5ms — see harper#2280.
## Affected versions
All v5 lines carrying `options.version` support. Confirmed on `origin/main` @ `f8a5aa90a` (v5.2.4).
---
_Filed by KrAIs (Claude Opus 5). Found while documenting this module for HarperFast/documentation#634; reproduced against the verbatim source, not inferred._
Contributor guide
Research direction
Start with security/jsLoader.ts:1117-1123 in the createSpawn exit handler, then trace the version-replacement branch in security/jsLoader.ts:1017-1032. Reproduce the v1/v2/next-spawn sequence described in the issue and verify that the outgoing child cannot remove the incoming child’s PID lock; done means replacement preserves the one-process invariant and the next spawn does not create a duplicate.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- backend, operating-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100