From f2dd8d8a515c1a2a220836231ad1a671a5e9b73d Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Mon, 13 Apr 2026 08:09:33 +0300 Subject: ask: serialize concurrent CLI with repo lock and stale PID recovery Add advisory lock under .git/hexai-ask.lock around Taskwarrior execution, with metadata (PID and process basename) and Linux /proc comm checks to remove orphan lock files when the recorded holder is gone or not ask. Extract internal/filelock for shared flock helpers; stats uses it too. Made-with: Cursor --- internal/askcli/runlock.go | 147 ++++++++++++++++++++++++++++ internal/askcli/runlock_stale_linux.go | 33 +++++++ internal/askcli/runlock_stale_linux_test.go | 19 ++++ internal/askcli/runlock_stale_other.go | 23 +++++ internal/askcli/runlock_test.go | 46 +++++++++ internal/askcli/taskexec.go | 14 ++- internal/askcli/taskexec_test.go | 34 +++++-- internal/filelock/filelock.go | 45 +++++++++ internal/filelock/filelock_test.go | 67 +++++++++++++ internal/filelock/lock_posix.go | 23 +++++ internal/filelock/lock_windows.go | 24 +++++ internal/stats/lock_posix.go | 23 ----- internal/stats/lock_windows.go | 24 ----- internal/stats/stats.go | 31 +----- internal/stats/stats_test.go | 8 +- 15 files changed, 473 insertions(+), 88 deletions(-) create mode 100644 internal/askcli/runlock.go create mode 100644 internal/askcli/runlock_stale_linux.go create mode 100644 internal/askcli/runlock_stale_linux_test.go create mode 100644 internal/askcli/runlock_stale_other.go create mode 100644 internal/askcli/runlock_test.go create mode 100644 internal/filelock/filelock.go create mode 100644 internal/filelock/filelock_test.go create mode 100644 internal/filelock/lock_posix.go create mode 100644 internal/filelock/lock_windows.go delete mode 100644 internal/stats/lock_posix.go delete mode 100644 internal/stats/lock_windows.go diff --git a/internal/askcli/runlock.go b/internal/askcli/runlock.go new file mode 100644 index 0000000..d0d7be3 --- /dev/null +++ b/internal/askcli/runlock.go @@ -0,0 +1,147 @@ +package askcli + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + "codeberg.org/snonux/hexai/internal/filelock" +) + +const askRepoLockFile = "hexai-ask.lock" + +var errAskLockReopen = errors.New("ask lock: reopen after stale file removal") + +func lockProcessLabel() string { + if exe, err := os.Executable(); err == nil { + if b := filepath.Base(exe); b != "" && b != "." { + return b + } + } + if b := filepath.Base(os.Args[0]); b != "" { + return b + } + return "ask" +} + +func readLockHolderPID(f *os.File) int { + if _, err := f.Seek(0, io.SeekStart); err != nil { + return 0 + } + var buf [64]byte + n, err := f.Read(buf[:]) + if err != nil && !errors.Is(err, io.EOF) { + return 0 + } + line := strings.TrimSpace(string(buf[:n])) + if line == "" { + return 0 + } + end := strings.IndexAny(line, "\n\r \t") + if end >= 0 { + line = line[:end] + } + pid, err := strconv.Atoi(line) + if err != nil || pid <= 0 { + return 0 + } + return pid +} + +func writeLockMetadata(f *os.File, pid int, comm string) error { + if _, err := f.Seek(0, io.SeekStart); err != nil { + return err + } + if err := f.Truncate(0); err != nil { + return err + } + _, err := fmt.Fprintf(f, "%d\n%s\n", pid, comm) + if err != nil { + return err + } + return f.Sync() +} + +// 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). +// errAskLockReopen means the caller should open the lock path again after stale removal. +func waitOrAcquireAskLockFD( + ctx context.Context, + f *os.File, + lockPath string, + comm string, + retryTimer *time.Timer, +) (func() error, error) { + for { + err := filelock.TryExclusive(f) + if err == nil { + if werr := writeLockMetadata(f, os.Getpid(), comm); werr != nil { + _ = filelock.UnlockExclusive(f) + _ = f.Close() + return nil, fmt.Errorf("ask lock: write metadata: %w", werr) + } + return func() error { + uErr := filelock.UnlockExclusive(f) + cErr := f.Close() + return errors.Join(uErr, cErr) + }, nil + } + if !errors.Is(err, filelock.ErrWouldBlock) { + _ = f.Close() + return nil, fmt.Errorf("ask lock: %w", err) + } + + pid := readLockHolderPID(f) + if pid > 0 && lockHolderIsStale(pid, comm) { + _ = f.Close() + if rerr := os.Remove(lockPath); rerr != nil && !errors.Is(rerr, os.ErrNotExist) { + return nil, fmt.Errorf("ask lock: remove stale %s: %w", lockPath, rerr) + } + return nil, errAskLockReopen + } + + retryTimer.Reset(5 * time.Millisecond) + select { + case <-ctx.Done(): + _ = f.Close() + return nil, ctx.Err() + case <-retryTimer.C: + } + } +} + +// acquireAskRepoLock serializes ask CLI access for a git working copy. It uses an +// advisory lock under .git and records holder PID plus process name for stale detection. +func acquireAskRepoLock(ctx context.Context, gitRoot string) (func() error, error) { + lockPath := filepath.Join(gitRoot, ".git", askRepoLockFile) + if err := os.MkdirAll(filepath.Dir(lockPath), 0o755); err != nil { + return nil, fmt.Errorf("ask lock: mkdir: %w", err) + } + + comm := lockProcessLabel() + retryTimer := time.NewTimer(5 * time.Millisecond) + defer retryTimer.Stop() + + for removalAttempts := 0; removalAttempts < 16; removalAttempts++ { + 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) + } + unlock, err := waitOrAcquireAskLockFD(ctx, f, lockPath, comm, retryTimer) + if err == nil { + return unlock, nil + } + if errors.Is(err, errAskLockReopen) { + continue + } + return nil, err + } + + return nil, fmt.Errorf("ask lock: could not acquire %s after stale recovery attempts", lockPath) +} diff --git a/internal/askcli/runlock_stale_linux.go b/internal/askcli/runlock_stale_linux.go new file mode 100644 index 0000000..183dfb2 --- /dev/null +++ b/internal/askcli/runlock_stale_linux.go @@ -0,0 +1,33 @@ +//go:build linux + +package askcli + +import ( + "os" + "strconv" + "strings" + + "golang.org/x/sys/unix" +) + +func lockHolderIsStale(pid int, expectedComm string) bool { + if pid <= 0 { + return false + } + if err := unix.Kill(pid, 0); err != nil { + return true + } + data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/comm") + if err != nil { + return false + } + holder := strings.TrimSpace(string(data)) + if holder == "" { + return false + } + want := expectedComm + if len(want) > 15 { + want = want[:15] + } + return holder != want +} diff --git a/internal/askcli/runlock_stale_linux_test.go b/internal/askcli/runlock_stale_linux_test.go new file mode 100644 index 0000000..7c581fb --- /dev/null +++ b/internal/askcli/runlock_stale_linux_test.go @@ -0,0 +1,19 @@ +//go:build linux + +package askcli + +import ( + "os/exec" + "testing" +) + +func TestLockHolderIsStale_NonAskLiveProcess(t *testing.T) { + cmd := exec.Command("sleep", "60") + if err := cmd.Start(); err != nil { + t.Skip("sleep not available:", err) + } + defer func() { _ = cmd.Process.Kill() }() + if !lockHolderIsStale(cmd.Process.Pid, "ask") { + t.Fatal("expected sleep process to be stale when expecting ask") + } +} diff --git a/internal/askcli/runlock_stale_other.go b/internal/askcli/runlock_stale_other.go new file mode 100644 index 0000000..21174c4 --- /dev/null +++ b/internal/askcli/runlock_stale_other.go @@ -0,0 +1,23 @@ +//go:build !linux + +package askcli + +import ( + "os" + "syscall" +) + +func lockHolderIsStale(pid int, expectedComm string) bool { + if pid <= 0 { + return false + } + _ = expectedComm + proc, err := os.FindProcess(pid) + if err != nil { + return true + } + if err := proc.Signal(syscall.Signal(0)); err != nil { + return true + } + return false +} diff --git a/internal/askcli/runlock_test.go b/internal/askcli/runlock_test.go new file mode 100644 index 0000000..f56f214 --- /dev/null +++ b/internal/askcli/runlock_test.go @@ -0,0 +1,46 @@ +package askcli + +import ( + "context" + "os" + "path/filepath" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestAcquireAskRepoLock_SerializesConcurrentHolders(t *testing.T) { + tmp := t.TempDir() + if err := os.MkdirAll(filepath.Join(tmp, ".git"), 0o755); err != nil { + t.Fatal(err) + } + var maxHeld int32 + var cur int32 + var wg sync.WaitGroup + for i := 0; i < 6; i++ { + wg.Add(1) + go func() { + defer wg.Done() + unlock, err := acquireAskRepoLock(context.Background(), tmp) + if err != nil { + t.Errorf("lock: %v", err) + return + } + defer func() { _ = unlock() }() + n := atomic.AddInt32(&cur, 1) + for { + old := atomic.LoadInt32(&maxHeld) + if n <= old || atomic.CompareAndSwapInt32(&maxHeld, old, n) { + break + } + } + time.Sleep(25 * time.Millisecond) + atomic.AddInt32(&cur, -1) + }() + } + wg.Wait() + if got := atomic.LoadInt32(&maxHeld); got != 1 { + t.Fatalf("max concurrent lock holders = %d, want 1", got) + } +} diff --git a/internal/askcli/taskexec.go b/internal/askcli/taskexec.go index 0b68e3b..4eed461 100644 --- a/internal/askcli/taskexec.go +++ b/internal/askcli/taskexec.go @@ -77,17 +77,25 @@ func (e Executor) Run(ctx context.Context, args []string, stdin io.Reader, stdou if err != nil { return 1, fmt.Errorf("%s: task binary lookup failed: %w", executor.label(), err) } + gitRoot, gitErr := executor.detectRepoRoot(ctx) repoRoot := "" if _, ok := taskProjectFromContext(ctx); !ok { - repoRoot, err = executor.detectRepoRoot(ctx) - if err != nil { - return 1, fmt.Errorf("%s: must be run inside a git repository: %w", executor.label(), err) + if gitErr != nil { + return 1, fmt.Errorf("%s: must be run inside a git repository: %w", executor.label(), gitErr) } + repoRoot = gitRoot } taskArgs, err := executor.taskArgs(ctx, repoRoot, args) if err != nil { return 1, fmt.Errorf("%s: %w", executor.label(), err) } + if gitErr == nil { + unlockAsk, lerr := acquireAskRepoLock(ctx, gitRoot) + if lerr != nil { + return 1, fmt.Errorf("%s: %w", executor.label(), lerr) + } + defer func() { _ = unlockAsk() }() + } if err := executor.runCommand(ctx, taskPath, taskArgs, stdin, stdout, stderr); err != nil { return exitCodeFor(err), nil } diff --git a/internal/askcli/taskexec_test.go b/internal/askcli/taskexec_test.go index 2236866..5e95f1c 100644 --- a/internal/askcli/taskexec_test.go +++ b/internal/askcli/taskexec_test.go @@ -5,12 +5,23 @@ import ( "context" "errors" "io" + "os" "os/exec" + "path/filepath" "reflect" "strings" "testing" ) +func fakeHexaiRepoDir(t *testing.T) string { + t.Helper() + base := filepath.Join(t.TempDir(), "hexai") + if err := os.MkdirAll(filepath.Join(base, ".git"), 0o755); err != nil { + t.Fatalf("mkdir .git: %v", err) + } + return base +} + func TestExecutorTaskArgs(t *testing.T) { exec_ := NewExecutor("ask") args, err := exec_.taskArgs(context.Background(), "/tmp/work/hexai", []string{"list", "limit:1"}) @@ -75,12 +86,13 @@ func TestExecutorTaskArgs_AddNoAgentScope(t *testing.T) { } func TestExecutorRun_InjectsProjectFilterAndAgentTag(t *testing.T) { + repo := fakeHexaiRepoDir(t) var gotName string var gotArgs []string exec_ := Executor{ commandName: "ask", findBinary: func() (string, error) { return "/usr/bin/task", nil }, - detectRepoRoot: func(context.Context) (string, error) { return "/tmp/work/hexai", nil }, + detectRepoRoot: func(context.Context) (string, error) { return repo, nil }, runCommand: func(_ context.Context, name string, args []string, stdin io.Reader, stdout, stderr io.Writer) error { gotName = name gotArgs = append([]string(nil), args...) @@ -105,11 +117,12 @@ func TestExecutorRun_InjectsProjectFilterAndAgentTag(t *testing.T) { } func TestExecutorRun_InjectsProjectFilterAndNoAgentTag(t *testing.T) { + repo := fakeHexaiRepoDir(t) var gotArgs []string exec_ := Executor{ commandName: "ask", findBinary: func() (string, error) { return "/usr/bin/task", nil }, - detectRepoRoot: func(context.Context) (string, error) { return "/tmp/work/hexai", nil }, + detectRepoRoot: func(context.Context) (string, error) { return repo, nil }, runCommand: func(_ context.Context, name string, args []string, stdin io.Reader, stdout, stderr io.Writer) error { gotArgs = append([]string(nil), args...) return nil @@ -130,14 +143,16 @@ func TestExecutorRun_InjectsProjectFilterAndNoAgentTag(t *testing.T) { } } -func TestExecutorRun_ProjectOverrideSkipsRepoDetection(t *testing.T) { +func TestExecutorRun_ProjectOverrideStillLocksUsingGitRoot(t *testing.T) { + repo := fakeHexaiRepoDir(t) + var detectCalls int var gotArgs []string exec_ := Executor{ commandName: "ask", findBinary: func() (string, error) { return "/usr/bin/task", nil }, detectRepoRoot: func(context.Context) (string, error) { - t.Fatal("detectRepoRoot should not be called when project override is set") - return "", nil + detectCalls++ + return repo, nil }, runCommand: func(_ context.Context, name string, args []string, stdin io.Reader, stdout, stderr io.Writer) error { gotArgs = append([]string(nil), args...) @@ -153,6 +168,9 @@ func TestExecutorRun_ProjectOverrideSkipsRepoDetection(t *testing.T) { if exitCode != 0 { t.Fatalf("exitCode = %d, want 0", exitCode) } + if detectCalls != 1 { + t.Fatalf("detectRepoRoot calls = %d, want 1", detectCalls) + } wantArgs := []string{"rc.verbose=nothing", "rc.confirmation=off", "project:alpha", "+agent", "list"} if !reflect.DeepEqual(gotArgs, wantArgs) { t.Fatalf("task args = %v, want %v", gotArgs, wantArgs) @@ -180,10 +198,11 @@ func TestExecutorRun_OutsideGitRepo_IsActionable(t *testing.T) { } func TestExecutorRun_PreservesTaskwarriorExitCode(t *testing.T) { + repo := fakeHexaiRepoDir(t) exec_ := Executor{ commandName: "ask", findBinary: func() (string, error) { return "/usr/bin/task", nil }, - detectRepoRoot: func(context.Context) (string, error) { return "/tmp/work/hexai", nil }, + detectRepoRoot: func(context.Context) (string, error) { return repo, nil }, runCommand: func(context.Context, string, []string, io.Reader, io.Writer, io.Writer) error { return exec.Command("sh", "-c", "exit 7").Run() }, @@ -199,12 +218,13 @@ func TestExecutorRun_PreservesTaskwarriorExitCode(t *testing.T) { } func TestExecutorRun_PreservesStdoutAndStderr(t *testing.T) { + repo := fakeHexaiRepoDir(t) var stdout bytes.Buffer var stderr bytes.Buffer exec_ := Executor{ commandName: "ask", findBinary: func() (string, error) { return "/usr/bin/task", nil }, - detectRepoRoot: func(context.Context) (string, error) { return "/tmp/work/hexai", nil }, + detectRepoRoot: func(context.Context) (string, error) { return repo, nil }, runCommand: func(_ context.Context, name string, args []string, stdin io.Reader, out, errOut io.Writer) error { _, _ = io.WriteString(out, "task stdout") _, _ = io.WriteString(errOut, "task stderr") diff --git a/internal/filelock/filelock.go b/internal/filelock/filelock.go new file mode 100644 index 0000000..192c3ca --- /dev/null +++ b/internal/filelock/filelock.go @@ -0,0 +1,45 @@ +// Package filelock provides advisory exclusive locks on open files. +package filelock + +import ( + "context" + "errors" + "os" + "time" +) + +// ErrWouldBlock indicates a non-blocking lock attempt could not acquire the lock. +var ErrWouldBlock = errors.New("filelock: would block") + +// TryExclusive attempts a non-blocking exclusive advisory lock on f. +func TryExclusive(f *os.File) error { + return tryLockExclusive(f.Fd()) +} + +// UnlockExclusive releases the advisory lock held on f. +func UnlockExclusive(f *os.File) error { + return unlockExclusive(f.Fd()) +} + +// 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) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-retryTimer.C: + } + continue + } + return nil, err + } +} diff --git a/internal/filelock/filelock_test.go b/internal/filelock/filelock_test.go new file mode 100644 index 0000000..f1f5b65 --- /dev/null +++ b/internal/filelock/filelock_test.go @@ -0,0 +1,67 @@ +package filelock + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" +) + +func TestTryExclusive_SecondDescriptorWouldBlock(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "lock") + f1, err := os.OpenFile(p, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = f1.Close() }) + f2, err := os.OpenFile(p, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = f2.Close() }) + if err := TryExclusive(f1); err != nil { + t.Fatalf("first TryExclusive: %v", err) + } + err = TryExclusive(f2) + if !errors.Is(err, ErrWouldBlock) { + t.Fatalf("second TryExclusive = %v, want ErrWouldBlock", err) + } + if err := UnlockExclusive(f1); err != nil { + t.Fatal(err) + } + if err := TryExclusive(f2); err != nil { + t.Fatalf("after unlock: %v", err) + } + if err := UnlockExclusive(f2); err != nil { + t.Fatal(err) + } +} + +func TestAcquireExclusive_ContextCancelledWhileBlocked(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "lock") + fHeld, err := os.OpenFile(p, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = fHeld.Close() }) + if err := TryExclusive(fHeld); err != nil { + t.Fatal(err) + } + fWait, err := os.OpenFile(p, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = fWait.Close() }) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err = AcquireExclusive(ctx, fWait) + if !errors.Is(err, context.Canceled) { + t.Fatalf("AcquireExclusive = %v, want context.Canceled", err) + } + if err := UnlockExclusive(fHeld); err != nil { + t.Fatal(err) + } +} diff --git a/internal/filelock/lock_posix.go b/internal/filelock/lock_posix.go new file mode 100644 index 0000000..5e49e3b --- /dev/null +++ b/internal/filelock/lock_posix.go @@ -0,0 +1,23 @@ +//go:build !windows + +package filelock + +import ( + "errors" + + "golang.org/x/sys/unix" +) + +func tryLockExclusive(fd uintptr) error { + if err := unix.Flock(int(fd), unix.LOCK_EX|unix.LOCK_NB); err != nil { + if errors.Is(err, unix.EWOULDBLOCK) || errors.Is(err, unix.EAGAIN) { + return ErrWouldBlock + } + return err + } + return nil +} + +func unlockExclusive(fd uintptr) error { + return unix.Flock(int(fd), unix.LOCK_UN) +} diff --git a/internal/filelock/lock_windows.go b/internal/filelock/lock_windows.go new file mode 100644 index 0000000..058b942 --- /dev/null +++ b/internal/filelock/lock_windows.go @@ -0,0 +1,24 @@ +//go:build windows + +package filelock + +import ( + "golang.org/x/sys/windows" +) + +func tryLockExclusive(fd uintptr) error { + var ol windows.Overlapped + err := windows.LockFileEx(windows.Handle(fd), windows.LOCKFILE_EXCLUSIVE_LOCK|windows.LOCKFILE_FAIL_IMMEDIATELY, 0, 1, 0, &ol) + if err == nil { + return nil + } + if err == windows.ERROR_LOCK_VIOLATION { + return ErrWouldBlock + } + return err +} + +func unlockExclusive(fd uintptr) error { + var ol windows.Overlapped + return windows.UnlockFileEx(windows.Handle(fd), 0, 1, 0, &ol) +} diff --git a/internal/stats/lock_posix.go b/internal/stats/lock_posix.go deleted file mode 100644 index 2c41d31..0000000 --- a/internal/stats/lock_posix.go +++ /dev/null @@ -1,23 +0,0 @@ -//go:build !windows - -package stats - -import ( - "errors" - - "golang.org/x/sys/unix" -) - -func tryLockFile(fd uintptr) error { - if err := unix.Flock(int(fd), unix.LOCK_EX|unix.LOCK_NB); err != nil { - if errors.Is(err, unix.EWOULDBLOCK) { - return errLockWouldBlock - } - return err - } - return nil -} - -func unlockFile(fd uintptr) error { - return unix.Flock(int(fd), unix.LOCK_UN) -} diff --git a/internal/stats/lock_windows.go b/internal/stats/lock_windows.go deleted file mode 100644 index 2ec5e90..0000000 --- a/internal/stats/lock_windows.go +++ /dev/null @@ -1,24 +0,0 @@ -//go:build windows - -package stats - -import ( - "golang.org/x/sys/windows" -) - -func tryLockFile(fd uintptr) error { - var ol windows.Overlapped - err := windows.LockFileEx(windows.Handle(fd), windows.LOCKFILE_EXCLUSIVE_LOCK|windows.LOCKFILE_FAIL_IMMEDIATELY, 0, 1, 0, &ol) - if err == nil { - return nil - } - if err == windows.ERROR_LOCK_VIOLATION { - return errLockWouldBlock - } - return err -} - -func unlockFile(fd uintptr) error { - var ol windows.Overlapped - return windows.UnlockFileEx(windows.Handle(fd), 0, 1, 0, &ol) -} diff --git a/internal/stats/stats.go b/internal/stats/stats.go index bd91e20..a5c5cf1 100644 --- a/internal/stats/stats.go +++ b/internal/stats/stats.go @@ -16,6 +16,8 @@ import ( "strings" "sync/atomic" "time" + + "codeberg.org/snonux/hexai/internal/filelock" ) const ( @@ -27,8 +29,6 @@ const ( var windowSeconds int64 = int64(defaultWindow.Seconds()) -var errLockWouldBlock = errors.New("stats: lock would block") - // nowFunc is the clock source for event timestamps and pruning cutoffs. // Replaced in tests to control time without sleeping. var nowFunc = time.Now @@ -141,7 +141,7 @@ func lockStatsFile(ctx context.Context, dir string) (func() error, error) { if err != nil { return nil, err } - unlock, err := acquireFileLock(ctx, f) + unlock, err := filelock.AcquireExclusive(ctx, f) if err != nil { _ = f.Close() return nil, err @@ -215,31 +215,6 @@ func writeStatsFileAtomic(dir, path string, sf *File) error { return nil } -// acquireFileLock spins on tryLockFile until it succeeds, the context is -// cancelled, or an unexpected error occurs. A single timer is reused across -// retries to avoid leaking timers/channels on every loop iteration. -func acquireFileLock(ctx context.Context, f *os.File) (func() error, error) { - fd := f.Fd() - retryTimer := time.NewTimer(5 * time.Millisecond) - defer retryTimer.Stop() - for { - err := tryLockFile(fd) - if err == nil { - return func() error { return unlockFile(fd) }, nil - } - if errors.Is(err, errLockWouldBlock) { - retryTimer.Reset(5 * time.Millisecond) - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-retryTimer.C: - } - continue - } - return nil, err - } -} - // TakeSnapshot reads the stats file and aggregates events within the stored // window (falling back to the process-level Window() if the file has none). // This is a pure read — it does not mutate global state. diff --git a/internal/stats/stats_test.go b/internal/stats/stats_test.go index 47e3068..fc043a5 100644 --- a/internal/stats/stats_test.go +++ b/internal/stats/stats_test.go @@ -9,6 +9,8 @@ import ( "sync" "testing" "time" + + "codeberg.org/snonux/hexai/internal/filelock" ) func TestUpdateAndSnapshot_Single(t *testing.T) { @@ -309,7 +311,7 @@ func TestTakeSnapshot_ZeroWindowSeconds(t *testing.T) { } // TestUpdate_CancelledContext covers the context cancellation branch in -// acquireFileLock when the lock is already held. +// filelock.AcquireExclusive when the lock is already held. func TestUpdate_CancelledContext(t *testing.T) { dir := t.TempDir() t.Setenv("XDG_CACHE_HOME", dir) @@ -320,14 +322,14 @@ func TestUpdate_CancelledContext(t *testing.T) { t.Fatal(err) } - // Hold the lock file to force acquireFileLock to spin. + // Hold the lock file to force filelock.AcquireExclusive to spin. lockPath := filepath.Join(statsDir, lockFileName) lf, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600) if err != nil { t.Fatal(err) } defer func() { _ = lf.Close() }() - unlock, err := acquireFileLock(context.Background(), lf) + unlock, err := filelock.AcquireExclusive(context.Background(), lf) if err != nil { t.Fatal(err) } -- cgit v1.2.3