fix(3670): break migration lock self-deadlock on Windows (#3765)
* test(installer): add regression tests for #3670 migration lock self-deadlock - T1: same-process PID re-entry reclamation (primary regression) - T2: dead-PID stale lock reclamation - T3: unlinkSync EPERM surfaces (not silently swallowed via force:true) - T4: counter-test — normal round-trip still works - T5: counter-test — genuinely-held live lock still errors clearly - Update existing 'reports lock release failures' test to mock fs.unlinkSync (not fs.rmSync) matching the fixed release path Windows wall-clock deadlock repro depends on Docker matrix Windows runners. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(installer): break migration lock self-deadlock on Windows (#3670) Root cause: `acquireInstallMigrationLock` release closure called `fs.rmSync(lockPath, { force: true })`. On Windows NTFS, a file recently closed via `closeSync(fd)` may still return EPERM from `unlink` until the OS fully releases the handle. The `force: true` flag silently swallows EPERM, leaving the lock file on disk. The subsequent `runInstallerMigrations` call in the same install() invocation hits EEXIST, spins for 30 s, then throws "installer migration lock is held". Fix: 1. Release closure uses `fs.unlinkSync` (not rmSync+force) so EPERM propagates via releaseError instead of being swallowed. 2. `acquireInstallMigrationLock` closes the fd before writing the payload (path-based write), eliminating the open handle that caused the deferred EPERM on Windows. 3. Stale-lock reclamation: on EEXIST, parse the on-disk PID and reclaim immediately if it matches process.pid (same-process re-entry, the primary #3670 failure mode) or if the PID is dead (ESRCH). Live alien PIDs still trigger the 30 s timeout. 4. Error message on a genuinely-held lock now includes the holder PID and acquiredAt timestamp for operator diagnostics. No public API change. All callers of runInstallerMigrations are inside installer-migrations.cjs and bin/install.js. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(3670): update changeset to reference PR #3765 * fix(3670): timeout on reclaim-unlink failure to prevent spin-loop regression When unlinkSync throws (e.g. Windows EPERM on an open handle) in the same-PID / dead-PID reclamation path, the original code continued unconditionally — bypassing the timeout check and reintroducing the exact deadlock the PR is supposed to fix. Guard the continue behind a `reclaimed` flag: only loop back to openSync if unlink SUCCEEDED. On failure, fall through to the existing bounded sleep + timeout, which surfaces "installer migration lock is held" within lockTimeoutMs instead of spinning indefinitely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(3670): tighten T5 — assert bounded failure when reclaim unlink fails on live lock The old T5 accepted BOTH success and throw, which allowed over-reclamation of a genuinely un-reclaimable lock to pass undetected. Rewrite T5 to force deterministically unreclaimable conditions: - Pre-seed lock with process.pid (triggers isSameProcess path) - Mock fs.unlinkSync via mock.method() to throw EPERM for the lock file With the production fix: reclaimed=false → falls through to timeout → throws "installer migration lock is held" within ~200ms. Without the production fix: unlink throws but continue runs anyway → process spins and eventually OOMs (confirmed RED: 136s runtime, V8 heap exhaustion from infinite readLockFile + new Error() allocations). assert.throws() now makes success a hard failure, closing the over-reclamation gap. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(3670): clean up orphan lock file when writeFileSync fails after closeSync If closeSync(fd) succeeds (fd=null) but the subsequent writeFileSync throws, the empty lock file was left on disk. readLockFile returns null for an empty/invalid-JSON file, so the stale-lock reclamation path skips it, causing the next acquire attempt to spin to timeout. Track ownership with lockCreatedByUs flag; add a second cleanup branch in the catch block for the fd-already-closed case. Also fix changeset body to use the bold-prefix format required by all other fragments in .changeset/. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/happy-pandas-glide.md
Normal file
5
.changeset/happy-pandas-glide.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3765
|
||||
---
|
||||
**`--cursor --local` install no longer self-deadlocks on `gsd-install-migration.lock` on Windows** — release closure now uses `unlinkSync` so NTFS EPERM surfaces rather than being silently swallowed; stale-lock reclamation detects same-process PID re-entry and dead PIDs, reclaiming immediately instead of spinning for 30 s; empty lock files left by mid-write failures are also cleaned up to prevent orphan stale locks. (#3670)
|
||||
@@ -223,6 +223,40 @@ function sleepSync(ms) {
|
||||
Atomics.wait(new Int32Array(buffer), 0, 0, ms);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a given PID is alive on the current host.
|
||||
* Uses process.kill(pid, 0) which works on POSIX and Windows (Node's
|
||||
* implementation maps it to OpenProcess + GetExitCodeProcess on win32).
|
||||
* Returns true if alive or permission-denied (live but not ours),
|
||||
* false if ESRCH (no such process).
|
||||
*/
|
||||
function isPidAlive(pid) {
|
||||
if (typeof pid !== 'number' || !Number.isFinite(pid) || pid <= 0) return false;
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
return true; // alive (or permission denied — treat as live)
|
||||
} catch (err) {
|
||||
return err.code !== 'ESRCH';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Try to read and parse the lock file JSON. Returns null on any error
|
||||
* (missing, invalid JSON, I/O failure).
|
||||
*/
|
||||
function readLockFile(lockPath) {
|
||||
try {
|
||||
const raw = fs.readFileSync(lockPath, 'utf8');
|
||||
const parsed = JSON.parse(raw);
|
||||
if (parsed && typeof parsed === 'object' && typeof parsed.pid === 'number') {
|
||||
return parsed;
|
||||
}
|
||||
return null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function acquireInstallMigrationLock(configDir, { timeoutMs = DEFAULT_LOCK_TIMEOUT_MS } = {}) {
|
||||
fs.mkdirSync(configDir, { recursive: true });
|
||||
const lockPath = path.join(configDir, INSTALL_MIGRATION_LOCK_NAME);
|
||||
@@ -230,16 +264,28 @@ function acquireInstallMigrationLock(configDir, { timeoutMs = DEFAULT_LOCK_TIMEO
|
||||
|
||||
while (true) {
|
||||
let fd = null;
|
||||
let lockCreatedByUs = false;
|
||||
try {
|
||||
fd = fs.openSync(lockPath, 'wx');
|
||||
fs.writeFileSync(fd, JSON.stringify({
|
||||
// Close the open descriptor before writing so the file handle is
|
||||
// released on Windows before the release closure unlinks it.
|
||||
// Write payload via writeFileSync with the path (not the fd) so we
|
||||
// don't hold an open fd across the lifetime of the lock.
|
||||
fs.closeSync(fd);
|
||||
fd = null;
|
||||
lockCreatedByUs = true; // we own the file; clean it up on any subsequent error
|
||||
fs.writeFileSync(lockPath, JSON.stringify({
|
||||
pid: process.pid,
|
||||
acquiredAt: new Date().toISOString(),
|
||||
}) + '\n');
|
||||
lockCreatedByUs = false; // release closure owns cleanup from here
|
||||
return () => {
|
||||
const failures = [];
|
||||
try { fs.closeSync(fd); } catch (error) { failures.push(error); }
|
||||
try { fs.rmSync(lockPath, { force: true }); } catch (error) { failures.push(error); }
|
||||
// Use unlinkSync (not rmSync with { force: true }) so EPERM errors
|
||||
// are NOT silently swallowed. On Windows, if the unlink fails
|
||||
// transiently, the error surfaces via releaseError so the caller
|
||||
// can observe and surface it rather than leaving a stale lock.
|
||||
try { fs.unlinkSync(lockPath); } catch (error) { failures.push(error); }
|
||||
if (failures.length > 0) {
|
||||
const releaseError = new Error(`failed to release installer migration lock: ${lockPath}`);
|
||||
releaseError.failures = failures;
|
||||
@@ -249,11 +295,38 @@ function acquireInstallMigrationLock(configDir, { timeoutMs = DEFAULT_LOCK_TIMEO
|
||||
} catch (error) {
|
||||
if (fd !== null) {
|
||||
try { fs.closeSync(fd); } catch { /* best-effort */ }
|
||||
try { fs.rmSync(lockPath, { force: true }); } catch { /* best-effort */ }
|
||||
try { fs.unlinkSync(lockPath); } catch { /* best-effort */ }
|
||||
fd = null;
|
||||
} else if (lockCreatedByUs) {
|
||||
// fd was closed but writeFileSync threw before we returned the release
|
||||
// closure — the empty lock file is still on disk and must be removed
|
||||
// so it does not orphan as an unreadable (empty/invalid JSON) stale lock.
|
||||
try { fs.unlinkSync(lockPath); } catch { /* best-effort */ }
|
||||
}
|
||||
if (error && error.code === 'EEXIST') {
|
||||
// Stale-lock reclamation: read the on-disk PID and check liveness.
|
||||
// If the PID is dead (ESRCH) or is our own process (same-process
|
||||
// re-entry caused by rmSync silently swallowing an unlink error on
|
||||
// a previous call in the same invocation — the root cause of #3670),
|
||||
// reclaim the lock by removing the stale file and retrying.
|
||||
const lockData = readLockFile(lockPath);
|
||||
if (lockData !== null) {
|
||||
const holderPid = lockData.pid;
|
||||
const isSameProcess = holderPid === process.pid;
|
||||
const isDeadProcess = !isPidAlive(holderPid);
|
||||
if (isSameProcess || isDeadProcess) {
|
||||
// Reclaim: remove the stale lock and loop back to openSync.
|
||||
// Only continue (retry) when unlink actually succeeds — a silent
|
||||
// continue on reclaim failure recreates the original deadlock:
|
||||
// the lock stays on disk and we spin indefinitely.
|
||||
let reclaimed = false;
|
||||
try { fs.unlinkSync(lockPath); reclaimed = true; } catch { /* unlink failed — fall through to timeout path */ }
|
||||
if (reclaimed) continue;
|
||||
}
|
||||
}
|
||||
if (Date.now() - started >= timeoutMs) {
|
||||
throw new Error(`installer migration lock is held: ${lockPath}`);
|
||||
const holderInfo = lockData ? ` (held by pid ${lockData.pid} since ${lockData.acquiredAt})` : '';
|
||||
throw new Error(`installer migration lock is held: ${lockPath}${holderInfo}`);
|
||||
}
|
||||
sleepSync(Math.min(50, Math.max(1, timeoutMs - (Date.now() - started))));
|
||||
continue;
|
||||
|
||||
323
tests/bug-3670-cursor-local-install-migration-lock.test.cjs
Normal file
323
tests/bug-3670-cursor-local-install-migration-lock.test.cjs
Normal file
@@ -0,0 +1,323 @@
|
||||
/**
|
||||
* Regression tests for issue #3670: --cursor --local install self-deadlocks
|
||||
* on gsd-install-migration.lock.
|
||||
*
|
||||
* Root cause: On Windows, `fs.rmSync(lockPath, { force: true })` in the lock
|
||||
* release closure silently swallows EPERM errors that NTFS returns when a
|
||||
* recently-closed file descriptor's handle has not yet been fully released by
|
||||
* the OS. The lock file is left on disk. The next `runInstallerMigrations`
|
||||
* call in the same install() invocation hits EEXIST, spins for
|
||||
* DEFAULT_LOCK_TIMEOUT_MS (30 s), then throws "installer migration lock is
|
||||
* held". There is also no stale-PID reclamation: if the lock names the
|
||||
* current process's PID, the helper should reclaim rather than spin.
|
||||
*
|
||||
* Windows wall-clock deadlock repro depends on Docker matrix Windows runners.
|
||||
* These tests reproduce the failure modes via mock-injected fs faults on any
|
||||
* platform (macOS/Linux/Windows). They fail deterministically WITHOUT the fix
|
||||
* and pass WITH it.
|
||||
*
|
||||
* Test plan:
|
||||
* T1 (same-process re-entry / stale-PID reclamation — primary regression)
|
||||
* Pre-seed the lock file with {pid: process.pid, ...}. Verify that a
|
||||
* runInstallerMigrations call reclaims the lock and succeeds rather than
|
||||
* spinning 30 s and throwing.
|
||||
*
|
||||
* T2 (dead-PID reclamation — cross-invocation stale lock)
|
||||
* Pre-seed the lock file with a PID known to be dead. Verify that acquire
|
||||
* reclaims rather than throws.
|
||||
*
|
||||
* T3 (silent rmSync swallow / Windows EPERM simulation)
|
||||
* Inject a fault that makes fs.rmSync throw EPERM for the lock file only
|
||||
* (simulating Windows NTFS delete-pending). Verify that the lock file IS
|
||||
* removed by an alternative path (or that the error propagates) — i.e.
|
||||
* verify that the fix does not silently leave the lock on disk.
|
||||
*
|
||||
* T4 (counter-test: normal single acquire/release round-trip still works)
|
||||
* No pre-seeded lock. One runInstallerMigrations call. Must succeed and
|
||||
* leave no lock file behind.
|
||||
*
|
||||
* T5 (counter-test: genuinely-held live lock still surfaces an error)
|
||||
* Pre-seed lock with a live PID (process.pid) AND simulate a lock that
|
||||
* has been "truly acquired" (fd still open). With lockTimeoutMs: 0 and a
|
||||
* truly un-reclaimable lock, must still throw with a useful message naming
|
||||
* the holder PID. (This guards against over-reclamation.)
|
||||
*
|
||||
* @see https://github.com/gsd-build/get-shit-done/issues/3670
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test, mock } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
|
||||
const {
|
||||
INSTALL_MIGRATION_LOCK_NAME,
|
||||
runInstallerMigrations,
|
||||
} = require('../get-shit-done/bin/lib/installer-migrations.cjs');
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
function createTempDir() {
|
||||
return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3670-'));
|
||||
}
|
||||
|
||||
function cleanup(dir) {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
function lockPath(dir) {
|
||||
return path.join(dir, INSTALL_MIGRATION_LOCK_NAME);
|
||||
}
|
||||
|
||||
function writeLockFile(dir, pid, acquiredAt) {
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
lockPath(dir),
|
||||
JSON.stringify({ pid, acquiredAt: acquiredAt || new Date().toISOString() }) + '\n',
|
||||
'utf8'
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Find a PID that is guaranteed to be dead on this host.
|
||||
* We probe a set of high candidate PIDs (far outside the running set) and
|
||||
* pick the first one for which process.kill(pid, 0) throws ESRCH.
|
||||
* Falls back to 99999 if the probe loop exhausts (extremely unlikely).
|
||||
*/
|
||||
function findDeadPid() {
|
||||
// Avoid process.pid ± small numbers — those could be live siblings.
|
||||
for (let candidate = 600000; candidate < 700000; candidate += 1000) {
|
||||
try {
|
||||
process.kill(candidate, 0);
|
||||
// Still alive (or permission denied but exists) — try next
|
||||
} catch (err) {
|
||||
if (err.code === 'ESRCH') return candidate;
|
||||
}
|
||||
}
|
||||
return 99999; // fallback: extremely unlikely to be a live PID
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// T1: Same-process re-entry — stale lock with current process.pid reclaimed
|
||||
// ---------------------------------------------------------------------------
|
||||
test('T1: reclaims stale lock that names the current process PID (same-process re-entry)', (t) => {
|
||||
const configDir = createTempDir();
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
// Pre-seed lock file with the CURRENT process's PID — exactly what happens
|
||||
// on Windows when rmSync swallows EPERM after the first runInstallerMigrations
|
||||
// call releases (or fails to release) the lock.
|
||||
writeLockFile(configDir, process.pid);
|
||||
|
||||
// Without the fix: this would spin for lockTimeoutMs then throw.
|
||||
// With the fix: detects own PID → reclaims → succeeds.
|
||||
// lockTimeoutMs: 200 (fail fast so the test doesn't hang for 30 s without fix)
|
||||
const result = runInstallerMigrations({
|
||||
configDir,
|
||||
migrations: [],
|
||||
lockTimeoutMs: 200,
|
||||
});
|
||||
|
||||
assert.ok(result, 'runInstallerMigrations must return a result object');
|
||||
// Lock file must be removed after the call completes.
|
||||
assert.equal(
|
||||
fs.existsSync(lockPath(configDir)),
|
||||
false,
|
||||
'lock file must not remain on disk after successful runInstallerMigrations'
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// T2: Dead-PID reclamation — cross-invocation stale lock
|
||||
// ---------------------------------------------------------------------------
|
||||
test('T2: reclaims stale lock whose PID is no longer alive', (t) => {
|
||||
const configDir = createTempDir();
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
const deadPid = findDeadPid();
|
||||
writeLockFile(configDir, deadPid);
|
||||
|
||||
const result = runInstallerMigrations({
|
||||
configDir,
|
||||
migrations: [],
|
||||
lockTimeoutMs: 200,
|
||||
});
|
||||
|
||||
assert.ok(result, 'runInstallerMigrations must return a result object');
|
||||
assert.equal(
|
||||
fs.existsSync(lockPath(configDir)),
|
||||
false,
|
||||
'lock file must not remain on disk after stale-PID reclamation'
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// T3: Windows EPERM simulation — unlinkSync failure surfaces (not silently swallowed)
|
||||
// ---------------------------------------------------------------------------
|
||||
test('T3: lock release does not silently leave lock file on disk when unlink fails (Windows EPERM simulation)', (t) => {
|
||||
const configDir = createTempDir();
|
||||
const originalUnlinkSync = fs.unlinkSync;
|
||||
|
||||
t.after(() => {
|
||||
fs.unlinkSync = originalUnlinkSync;
|
||||
cleanup(configDir);
|
||||
});
|
||||
|
||||
// The fix uses fs.unlinkSync (not fs.rmSync with { force: true }) in the
|
||||
// release closure. Inject EPERM on the lock file to simulate the Windows
|
||||
// NTFS condition where the recently-closed handle has not been fully
|
||||
// released by the OS.
|
||||
//
|
||||
// The fix's contract: EPERM must NOT be silently swallowed.
|
||||
// Either (a) the error propagates as a releaseError, or (b) some alternative
|
||||
// deletion path succeeds. Silent-swallow (no error + file still exists) is
|
||||
// the failure condition we guard against.
|
||||
let unlinkCallCount = 0;
|
||||
fs.unlinkSync = function faultInjectUnlinkSync(targetPath) {
|
||||
const isLock = path.basename(String(targetPath)) === INSTALL_MIGRATION_LOCK_NAME;
|
||||
if (isLock) {
|
||||
unlinkCallCount++;
|
||||
// Simulate Windows EPERM (file handle not fully released by OS)
|
||||
const err = Object.assign(
|
||||
new Error('EPERM: operation not permitted, unlink ' + targetPath),
|
||||
{ code: 'EPERM' }
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
return originalUnlinkSync.call(fs, targetPath);
|
||||
};
|
||||
|
||||
// With the fix: unlinkSync throws EPERM → releaseError is thrown by the
|
||||
// release closure → runInstallerMigrations throws releaseError.
|
||||
// With the buggy code (rmSync + force:true): EPERM was swallowed silently,
|
||||
// no error thrown, lock file left on disk.
|
||||
//
|
||||
// Assert: if the call succeeds (no throw), the lock file must be gone.
|
||||
// If the call throws, the error message must reference the lock.
|
||||
let threw = false;
|
||||
let thrownError = null;
|
||||
try {
|
||||
runInstallerMigrations({
|
||||
configDir,
|
||||
migrations: [],
|
||||
lockTimeoutMs: 500,
|
||||
});
|
||||
} catch (err) {
|
||||
threw = true;
|
||||
thrownError = err;
|
||||
}
|
||||
|
||||
if (threw) {
|
||||
// Acceptable: error surfaced. Verify it's lock-related (not a bug elsewhere).
|
||||
assert.match(
|
||||
thrownError.message,
|
||||
/lock/i,
|
||||
'thrown error must reference the lock file'
|
||||
);
|
||||
} else {
|
||||
// If no error was thrown, the lock file must have been removed by some
|
||||
// alternative path (not left silently on disk).
|
||||
assert.equal(
|
||||
fs.existsSync(lockPath(configDir)),
|
||||
false,
|
||||
'if unlinkSync EPERM is encountered but no error thrown, lock file must still be removed'
|
||||
);
|
||||
}
|
||||
|
||||
// Sanity: the fault injection was actually triggered.
|
||||
assert.ok(unlinkCallCount > 0, 'unlinkSync must have been called for the lock file at least once');
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// T4: Counter-test — normal single acquire/release round-trip still works
|
||||
// ---------------------------------------------------------------------------
|
||||
test('T4: normal (non-recursive) runInstallerMigrations acquires and releases lock correctly', (t) => {
|
||||
const configDir = createTempDir();
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
// No pre-seeded lock. Standard happy path.
|
||||
const result = runInstallerMigrations({
|
||||
configDir,
|
||||
migrations: [],
|
||||
});
|
||||
|
||||
assert.ok(result, 'runInstallerMigrations must return a result');
|
||||
assert.equal(
|
||||
fs.existsSync(lockPath(configDir)),
|
||||
false,
|
||||
'lock file must be cleaned up after normal completion'
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// T5: Counter-test — unreclaimable live lock must surface a bounded error
|
||||
// ---------------------------------------------------------------------------
|
||||
// This test guards against over-reclamation: if the reclaim-unlink fails
|
||||
// (e.g. Windows EPERM on a live open handle), the fix must NOT spin
|
||||
// indefinitely — it must fall through to the timeout path and throw.
|
||||
//
|
||||
// Conditions forced by this test:
|
||||
// 1. Lock file contains the CURRENT process.pid (triggers isSameProcess branch).
|
||||
// 2. fs.unlinkSync is mocked to throw EPERM for the lock file (reclaim fails).
|
||||
// 3. lockTimeoutMs: 200 — timeout must fire within a short wall-clock window.
|
||||
//
|
||||
// Expected outcome: throws with /installer migration lock is held/ within
|
||||
// ~200ms. SUCCESS (no throw) is NOT acceptable here — that would mean the fix
|
||||
// over-reclaimed a lock that it couldn't actually remove.
|
||||
test('T5: unreclaimable same-PID lock throws bounded error (reclaim-unlink failure falls through to timeout)', (t) => {
|
||||
const configDir = createTempDir();
|
||||
const originalUnlinkSync = fs.unlinkSync;
|
||||
|
||||
t.after(() => {
|
||||
mock.restoreAll();
|
||||
fs.unlinkSync = originalUnlinkSync;
|
||||
cleanup(configDir);
|
||||
});
|
||||
|
||||
// Pre-seed lock file with the CURRENT process's PID.
|
||||
// This triggers the isSameProcess reclamation path inside acquireInstallerMigrationLock.
|
||||
writeLockFile(configDir, process.pid);
|
||||
|
||||
// Mock unlinkSync to throw EPERM for the lock file only.
|
||||
// This simulates Windows NTFS refusing to delete a file with an open handle.
|
||||
// With the fix: reclaim-unlink fails → reclaimed=false → falls through to
|
||||
// the timeout check → throws "installer migration lock is held" after ≤200ms.
|
||||
// Without the fix (original code): unlink throws but continue runs anyway →
|
||||
// spins indefinitely, never reaches the timeout check → deadlock.
|
||||
mock.method(fs, 'unlinkSync', function faultInjectUnlinkSync(targetPath) {
|
||||
const isLock = path.basename(String(targetPath)) === INSTALL_MIGRATION_LOCK_NAME;
|
||||
if (isLock) {
|
||||
const err = Object.assign(
|
||||
new Error('EPERM: operation not permitted, unlink ' + targetPath),
|
||||
{ code: 'EPERM' }
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
return originalUnlinkSync.call(fs, targetPath);
|
||||
});
|
||||
|
||||
const startMs = Date.now();
|
||||
assert.throws(
|
||||
() => runInstallerMigrations({
|
||||
configDir,
|
||||
migrations: [],
|
||||
lockTimeoutMs: 200,
|
||||
}),
|
||||
(err) => {
|
||||
assert.match(err.message, /installer migration lock is held/, 'error must name the held lock');
|
||||
return true;
|
||||
},
|
||||
'must throw "installer migration lock is held" when reclaim-unlink fails — not spin indefinitely'
|
||||
);
|
||||
const elapsedMs = Date.now() - startMs;
|
||||
|
||||
// Must resolve within a generous but finite window (fix + timeout overhead).
|
||||
// If it spins (deadlock regression), the process-level test timeout fires instead.
|
||||
t.diagnostic(`T5 elapsed: ${elapsedMs}ms (expected ≤500ms for lockTimeoutMs:200 + overhead)`);
|
||||
assert.ok(elapsedMs < 500, `T5 must resolve within 500ms; actual ${elapsedMs}ms — possible spin-loop regression`);
|
||||
});
|
||||
@@ -681,17 +681,20 @@ test('refuses to run migrations while another installer owns the migration lock'
|
||||
|
||||
test('reports lock release failures after migration work completes', (t) => {
|
||||
const configDir = createTempInstall();
|
||||
const originalRmSync = fs.rmSync;
|
||||
const originalUnlinkSync = fs.unlinkSync;
|
||||
t.after(() => {
|
||||
fs.rmSync = originalRmSync;
|
||||
fs.unlinkSync = originalUnlinkSync;
|
||||
cleanup(configDir);
|
||||
});
|
||||
|
||||
fs.rmSync = (targetPath, ...args) => {
|
||||
// The release closure uses fs.unlinkSync (not fs.rmSync) so that EPERM is
|
||||
// NOT silently swallowed on Windows (#3670). Mock unlinkSync to simulate
|
||||
// a Windows NTFS EPERM condition when the lock file is removed.
|
||||
fs.unlinkSync = (targetPath) => {
|
||||
if (path.basename(String(targetPath)) === 'gsd-install-migration.lock') {
|
||||
throw new Error('simulated lock unlink failure');
|
||||
}
|
||||
return originalRmSync.call(fs, targetPath, ...args);
|
||||
return originalUnlinkSync.call(fs, targetPath);
|
||||
};
|
||||
|
||||
assert.throws(
|
||||
|
||||
Reference in New Issue
Block a user