HarperFast / HarperFast/harper

Version-replacement handoff is not graceful: SIGTERM then a 5ms busy-wait before respawn, so a sidecar holding an exclusive resource fails to start with no retry

Open
#2,280 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

**Location:** `security/jsLoader.ts:1017-1032` (version-replacement branch in `acquirePidFileLock`)

## Impact

The `options.version` handoff sends SIGTERM and then starts the replacement ~5ms later without waiting for the old process to exit. A sidecar that holds a listening socket, a lock file, or any other exclusive resource can fail on startup — `EADDRINUSE` is the obvious case — and there is no retry: the caller gets a child that dies immediately.

## Details

```js
if (requestedVersion != null && requestedVersion !== existingVersion) {
try { process.kill(existingPid); } catch { } // SIGTERM, no wait
try { unlinkSync(pidFilePath); } catch { }
const start = Date.now();
while (Date.now() - start < retryDelay) { // busy-wait, retryDelay = 5ms
}
continue; // -> lock acquired -> spawn replacement
}
```

`retryDelay` defaults to `5`. There is no `waitpid`, no poll on `isProcessRunning(existingPid)` (which the module already has, at `security/jsLoader.ts:970`), and no readiness or retry on the spawned replacement. The busy-wait also burns the thread rather than yielding.

5ms is not enough for a process to run its SIGTERM handler, flush, and close a listening socket. For a sidecar that ignores SIGTERM entirely it is never enough.

## Reproduction

```
--- CLAIM 3: is the outgoing process still alive when the replacement spawns? ---
old pid 59797 SIGTERMed; new pid 59806 spawned 5.6ms later
old process still alive at the instant the replacement started? true
(no waitpid / no readiness check / no retry between the two)
```

## Recommended fix

Poll `isProcessRunning(existingPid)` with a bounded deadline before acquiring the lock, escalating SIGTERM → SIGKILL if the deadline passes, and yield rather than busy-wait. The module already has the liveness primitive; the handoff just doesn't use it.

Related: harper#2076 is the same class of defect (termination confirmation) in the *component install/deploy* spawn path in `components/Application.ts`, but with the opposite failure — an unbounded poll. A shared, bounded "wait for confirmed termination" helper would serve both.

## Priority note

Filed P3 deliberately: real, but nobody will schedule it on its own. It should be picked up by whoever rewrites this handoff for the double-process defect in harper#2279, which is why they are cross-linked.

## 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._

Contributor guide

Open the contributing guide

Research direction

Start in security/jsLoader.ts:1017-1032 and read the existing isProcessRunning primitive at line 970. Reproduce the version-replacement handoff, then implement and verify bounded termination confirmation, escalation when the deadline passes, and yielding instead of the 5ms busy-wait.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.