[object Object]

← back to Cli Printing Press

fix(cli): reclaim locks from dead owners (#564)

34413e94ccb06fd0824f78dfce89bd1b887b8d02 · 2026-05-03 21:49:43 -0700 · Trevin Chow

Files touched

Diff

commit 34413e94ccb06fd0824f78dfce89bd1b887b8d02
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sun May 3 21:49:43 2026 -0700

    fix(cli): reclaim locks from dead owners (#564)
---
 internal/pipeline/lock.go                     | 14 ++++++--
 internal/pipeline/lock_test.go                | 48 +++++++++++++++++++++++++++
 internal/pipeline/process_liveness_other.go   |  7 ++++
 internal/pipeline/process_liveness_unix.go    | 14 ++++++++
 internal/pipeline/process_liveness_windows.go | 32 ++++++++++++++++++
 5 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/internal/pipeline/lock.go b/internal/pipeline/lock.go
index fd5c639b..8cd8b08a 100644
--- a/internal/pipeline/lock.go
+++ b/internal/pipeline/lock.go
@@ -351,11 +351,21 @@ func validatePhase5GateForPromote(workingDir string, state *PipelineState) error
 	return fmt.Errorf("phase5 gate failed: %s", result.Detail)
 }
 
-// IsStale returns true if the lock's UpdatedAt is older than StaleLockThreshold.
+// IsStale returns true if the lock's heartbeat is too old or its owner
+// process is known to have exited.
 func IsStale(lock *LockState) bool {
-	return time.Since(lock.UpdatedAt) > StaleLockThreshold
+	if time.Since(lock.UpdatedAt) > StaleLockThreshold {
+		return true
+	}
+	return lockOwnerDead(lock)
+}
+
+func lockOwnerDead(lock *LockState) bool {
+	return lock.PID > 0 && !lockOwnerAliveFunc(lock.PID)
 }
 
+var lockOwnerAliveFunc = lockOwnerAlive
+
 func readLock(path string) (*LockState, error) {
 	data, err := os.ReadFile(path)
 	if err != nil {
diff --git a/internal/pipeline/lock_test.go b/internal/pipeline/lock_test.go
index f27e0169..5d078378 100644
--- a/internal/pipeline/lock_test.go
+++ b/internal/pipeline/lock_test.go
@@ -81,6 +81,25 @@ func TestAcquireLock_StaleLockAutoReclaim(t *testing.T) {
 	assert.Equal(t, "new-scope", lock.Scope)
 }
 
+func TestAcquireLock_FreshLockDeadPIDAutoReclaim(t *testing.T) {
+	setupLockTest(t)
+	setLockOwnerAliveForTest(t, false)
+
+	require.NoError(t, os.MkdirAll(LocksDir(), 0o755))
+	deadLock := &LockState{
+		Scope:      "old-scope",
+		Phase:      "build",
+		PID:        12345,
+		AcquiredAt: time.Now(),
+		UpdatedAt:  time.Now(),
+	}
+	require.NoError(t, writeLock(LockFilePath("test-pp-cli"), deadLock))
+
+	lock, err := AcquireLock("test-pp-cli", "new-scope", false)
+	require.NoError(t, err)
+	assert.Equal(t, "new-scope", lock.Scope)
+}
+
 func TestAcquireLock_FreshLockDifferentScope_Blocked(t *testing.T) {
 	setupLockTest(t)
 
@@ -145,6 +164,27 @@ func TestLockStatus_ActiveLock(t *testing.T) {
 	assert.NotNil(t, status.Lock)
 }
 
+func TestLockStatus_FreshLockDeadPIDIsStale(t *testing.T) {
+	setupLockTest(t)
+	setLockOwnerAliveForTest(t, false)
+
+	require.NoError(t, os.MkdirAll(LocksDir(), 0o755))
+	deadLock := &LockState{
+		Scope:      "old-scope",
+		Phase:      "build",
+		PID:        12345,
+		AcquiredAt: time.Now(),
+		UpdatedAt:  time.Now(),
+	}
+	require.NoError(t, writeLock(LockFilePath("test-pp-cli"), deadLock))
+
+	status := LockStatus("test-pp-cli")
+	assert.True(t, status.Held)
+	assert.True(t, status.Stale)
+	assert.Equal(t, "build", status.Phase)
+	assert.Equal(t, "old-scope", status.Scope)
+}
+
 func TestLockStatus_NoLock(t *testing.T) {
 	setupLockTest(t)
 
@@ -607,3 +647,11 @@ func TestConcurrentAcquire(t *testing.T) {
 	}
 	assert.GreaterOrEqual(t, winners, 1, "at least one goroutine should acquire the lock")
 }
+
+func setLockOwnerAliveForTest(t *testing.T, alive bool) {
+	t.Helper()
+
+	original := lockOwnerAliveFunc
+	lockOwnerAliveFunc = func(pid int) bool { return alive }
+	t.Cleanup(func() { lockOwnerAliveFunc = original })
+}
diff --git a/internal/pipeline/process_liveness_other.go b/internal/pipeline/process_liveness_other.go
new file mode 100644
index 00000000..0806dbfd
--- /dev/null
+++ b/internal/pipeline/process_liveness_other.go
@@ -0,0 +1,7 @@
+//go:build !unix && !windows
+
+package pipeline
+
+func lockOwnerAlive(pid int) bool {
+	return pid > 0
+}
diff --git a/internal/pipeline/process_liveness_unix.go b/internal/pipeline/process_liveness_unix.go
new file mode 100644
index 00000000..22b07a0e
--- /dev/null
+++ b/internal/pipeline/process_liveness_unix.go
@@ -0,0 +1,14 @@
+//go:build unix
+
+package pipeline
+
+import "syscall"
+
+func lockOwnerAlive(pid int) bool {
+	if pid <= 0 {
+		return false
+	}
+
+	err := syscall.Kill(pid, 0)
+	return err == nil || err == syscall.EPERM
+}
diff --git a/internal/pipeline/process_liveness_windows.go b/internal/pipeline/process_liveness_windows.go
new file mode 100644
index 00000000..04ad67c3
--- /dev/null
+++ b/internal/pipeline/process_liveness_windows.go
@@ -0,0 +1,32 @@
+//go:build windows
+
+package pipeline
+
+import (
+	"errors"
+
+	"golang.org/x/sys/windows"
+)
+
+const windowsStillActive = 259
+
+func lockOwnerAlive(pid int) bool {
+	if pid <= 0 {
+		return false
+	}
+
+	handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
+	if err != nil {
+		if errors.Is(err, windows.ERROR_INVALID_PARAMETER) {
+			return false
+		}
+		return errors.Is(err, windows.ERROR_ACCESS_DENIED)
+	}
+	defer windows.CloseHandle(handle)
+
+	var exitCode uint32
+	if err := windows.GetExitCodeProcess(handle, &exitCode); err != nil {
+		return true
+	}
+	return exitCode == windowsStillActive
+}

← c0639cf0 fix(cli): exempt generated-client source helpers (#563)  ·  back to Cli Printing Press  ·  fix(cli): parse epoch Retry-After headers (#565) e588bf7e →