From 6971543d4749574d4ca575c4e8acf04f9e86d6bb Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Thu, 17 Sep 2026 13:34:20 +0200 Subject: the directory lock is the kernel's, not a pid we believe flock(2) on the lock file's descriptor replaces "write my pid, and decide whether the pid in the file is still alive". The kernel drops the lock when the process ends, however it ends, so there is no stale krino lock to detect and no takeover to race over. What that fixes: - Two runs that both judged a lock stale could remove and recreate it and both believe they held it. Remove-then-create cannot be made atomic; there is nothing to make atomic now. Pinned by a test with eight callers over twenty rounds. - A pid reused after a crash made the lock live for ever, and the message named neither the file nor the pid, so there was nothing to act on. The message now names both. - Signal(0) reads EPERM as "not running", so a lock held by another user was taken over. There is no such judgement left to get wrong. A run that waits for a held lock now says so first. Waiting is what the spec asks for, but the wait has no timeout, and in silence it is indistinguishable from a hang - I spent two minutes on one myself today, waiting on a lock the window was holding. Tests that faked a held lock by writing a file now hold a real one. --- cmd/krino/matching_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'cmd/krino/matching_test.go') diff --git a/cmd/krino/matching_test.go b/cmd/krino/matching_test.go index 4ce2205..6bdc994 100644 --- a/cmd/krino/matching_test.go +++ b/cmd/krino/matching_test.go @@ -4,13 +4,15 @@ package main import ( "bytes" + "context" "encoding/json" - "fmt" "os" "path/filepath" "strings" "testing" "time" + + "git.labunix.xyz/krino/internal/lock" ) const dlRules = ` @@ -412,13 +414,13 @@ func TestRefusesWithoutTerminalAndWithoutFlags(t *testing.T) { func TestSecondRunFailsImmediatelyWithYes(t *testing.T) { h := matchingFixture(t) held := filepath.Join(h, ".local", "state", "krino", "dl.lock") - if err := os.MkdirAll(filepath.Dir(held), 0o755); err != nil { - t.Fatal(err) - } - // A lock held by this very process, so it is not stale. - if err := os.WriteFile(held, []byte(fmt.Sprintf("pid %d\n", os.Getpid())), 0o644); err != nil { + // A lock really held: it is the kernel's, so a file that merely looks + // like one holds nothing. + l, err := lock.Acquire(context.Background(), held, false) + if err != nil { t.Fatal(err) } + defer l.Release() code, _, errOut := runCLI(t, "-y") if code != 1 || !strings.Contains(errOut, "another krino") { t.Errorf("-y against a held lock: %d %q", code, errOut) -- cgit v1.3