summaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
Diffstat (limited to 'internal')
-rw-r--r--internal/askcli/runlock.go16
-rw-r--r--internal/askcli/runlock_test.go30
-rw-r--r--internal/filelock/filelock.go13
3 files changed, 49 insertions, 10 deletions
diff --git a/internal/askcli/runlock.go b/internal/askcli/runlock.go
index 9fbe0cd..f6a3cc6 100644
--- a/internal/askcli/runlock.go
+++ b/internal/askcli/runlock.go
@@ -66,13 +66,15 @@ func writeLockMetadata(f *os.File, pid int, comm string) error {
return f.Sync()
}
+// askLockRetryInterval is the backoff between successive non-blocking lock attempts.
+const askLockRetryInterval = 5 * time.Millisecond
+
// waitOrAcquireAskLockFD tries to take an exclusive lock on f, or blocks until ctx ends.
// On success it writes lock metadata and returns an unlock function (which closes f).
func waitOrAcquireAskLockFD(
ctx context.Context,
f *os.File,
comm string,
- retryTimer *time.Timer,
) (func() error, error) {
for {
err := filelock.TryExclusive(f)
@@ -100,12 +102,16 @@ func waitOrAcquireAskLockFD(
// Intentional no-op: contention is resolved only by waiting for flock release.
}
- retryTimer.Reset(5 * time.Millisecond)
+ // Use a fresh timer per iteration via time.After instead of reusing and
+ // Reset()-ing a single timer. Reset() on a timer that may still be pending is
+ // the documented Go timer hazard: a stale value can already be queued on the
+ // channel and trigger a spurious early wake-up. A new timer each loop guarantees
+ // a clean, full askLockRetryInterval delay (or ctx cancellation).
select {
case <-ctx.Done():
_ = f.Close()
return nil, ctx.Err()
- case <-retryTimer.C:
+ case <-time.After(askLockRetryInterval):
}
}
}
@@ -119,12 +125,10 @@ func acquireAskRepoLock(ctx context.Context, gitRoot string) (func() error, erro
}
comm := lockProcessLabel()
- retryTimer := time.NewTimer(5 * time.Millisecond)
- defer retryTimer.Stop()
f, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600)
if err != nil {
return nil, fmt.Errorf("ask lock: open %s: %w", lockPath, err)
}
- return waitOrAcquireAskLockFD(ctx, f, comm, retryTimer)
+ return waitOrAcquireAskLockFD(ctx, f, comm)
}
diff --git a/internal/askcli/runlock_test.go b/internal/askcli/runlock_test.go
index 8ef8f2c..c8266f0 100644
--- a/internal/askcli/runlock_test.go
+++ b/internal/askcli/runlock_test.go
@@ -91,6 +91,36 @@ func TestAcquireAskRepoLock_StaleMetadataDoesNotRotateContendedLockFile(t *testi
}
}
+// TestAcquireAskRepoLock_ContextCancelledWhileBlocked verifies the retry loop
+// honors context cancellation while it is parked on the per-iteration timer.
+// This guards the time.After-based wait that replaced the unsafe timer reuse:
+// cancellation must win the select and return ctx.Err() promptly.
+func TestAcquireAskRepoLock_ContextCancelledWhileBlocked(t *testing.T) {
+ tmp := t.TempDir()
+ holder, _, _ := prepareContendedStaleLock(t, tmp)
+ defer releaseContendedLock(t, holder)
+
+ ctx, cancel := context.WithCancel(context.Background())
+ resultCh := acquireLockAsync(ctx, tmp)
+
+ // Let the contender enter the retry loop, then cancel while it waits.
+ time.Sleep(20 * time.Millisecond)
+ cancel()
+
+ select {
+ case result := <-resultCh:
+ if result.unlock != nil {
+ _ = result.unlock()
+ t.Fatal("lock acquired despite cancellation")
+ }
+ if result.err != context.Canceled {
+ t.Fatalf("err = %v, want context.Canceled", result.err)
+ }
+ case <-time.After(time.Second):
+ t.Fatal("acquireAskRepoLock did not return after cancellation")
+ }
+}
+
func prepareContendedStaleLock(t *testing.T, gitRoot string) (*os.File, string, os.FileInfo) {
t.Helper()
lockDir := filepath.Join(gitRoot, ".git")
diff --git a/internal/filelock/filelock.go b/internal/filelock/filelock.go
index 192c3ca..bc3ae8a 100644
--- a/internal/filelock/filelock.go
+++ b/internal/filelock/filelock.go
@@ -21,22 +21,27 @@ func UnlockExclusive(f *os.File) error {
return unlockExclusive(f.Fd())
}
+// retryInterval is the backoff between successive non-blocking lock attempts.
+const retryInterval = 5 * time.Millisecond
+
// AcquireExclusive spins with TryExclusive until the lock is acquired, ctx is done, or a non-would-block error occurs.
func AcquireExclusive(ctx context.Context, f *os.File) (func() error, error) {
fd := f.Fd()
- retryTimer := time.NewTimer(5 * time.Millisecond)
- defer retryTimer.Stop()
for {
err := tryLockExclusive(fd)
if err == nil {
return func() error { return unlockExclusive(fd) }, nil
}
if errors.Is(err, ErrWouldBlock) {
- retryTimer.Reset(5 * time.Millisecond)
+ // Use a fresh timer per iteration via time.After instead of reusing and
+ // Reset()-ing a single timer. Resetting a timer that may still be pending is
+ // the documented hazard in the Go timer API: a stale value can already be
+ // queued on the channel, causing a spurious early wake-up. A new timer each
+ // loop guarantees a clean, full retryInterval delay (or ctx cancellation).
select {
case <-ctx.Done():
return nil, ctx.Err()
- case <-retryTimer.C:
+ case <-time.After(retryInterval):
}
continue
}