Version replacement leaves two sidecar processes running: the outgoing child's exit handler unlinks the incoming child's PID lock

[kriszyp]

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:

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 = <ABSENT>
  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:

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.