internal/app/state: use internal/lockedfile

This is a pretty solid implementation backed by robust tests, with a much cleaner interface.

Signed-off-by: Ophestra <cat@gensokyo.uk>

internal/lockedfile/internal/filelock/filelock.go

@@ -0,0 +1,83 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package filelock provides a platform-independent API for advisory file
+// locking. Calls to functions in this package on platforms that do not support
+// advisory locks will return errors for which IsNotSupported returns true.
+package filelock
+
+import (
+	"errors"
+	"io/fs"
+)
+
+// A File provides the minimal set of methods required to lock an open file.
+// File implementations must be usable as map keys.
+// The usual implementation is *os.File.
+type File interface {
+	// Name returns the name of the file.
+	Name() string
+
+	// Fd returns a valid file descriptor.
+	// (If the File is an *os.File, it must not be closed.)
+	Fd() uintptr
+
+	// Stat returns the FileInfo structure describing file.
+	Stat() (fs.FileInfo, error)
+}
+
+// Lock places an advisory write lock on the file, blocking until it can be
+// locked.
+//
+// If Lock returns nil, no other process will be able to place a read or write
+// lock on the file until this process exits, closes f, or calls Unlock on it.
+//
+// If f's descriptor is already read- or write-locked, the behavior of Lock is
+// unspecified.
+//
+// Closing the file may or may not release the lock promptly. Callers should
+// ensure that Unlock is always called when Lock succeeds.
+func Lock(f File) error {
+	return lock(f, writeLock)
+}
+
+// RLock places an advisory read lock on the file, blocking until it can be locked.
+//
+// If RLock returns nil, no other process will be able to place a write lock on
+// the file until this process exits, closes f, or calls Unlock on it.
+//
+// If f is already read- or write-locked, the behavior of RLock is unspecified.
+//
+// Closing the file may or may not release the lock promptly. Callers should
+// ensure that Unlock is always called if RLock succeeds.
+func RLock(f File) error {
+	return lock(f, readLock)
+}
+
+// Unlock removes an advisory lock placed on f by this process.
+//
+// The caller must not attempt to unlock a file that is not locked.
+func Unlock(f File) error {
+	return unlock(f)
+}
+
+// String returns the name of the function corresponding to lt
+// (Lock, RLock, or Unlock).
+func (lt lockType) String() string {
+	switch lt {
+	case readLock:
+		return "RLock"
+	case writeLock:
+		return "Lock"
+	default:
+		return "Unlock"
+	}
+}
+
+// IsNotSupported returns a boolean indicating whether the error is known to
+// report that a function is not supported (possibly for a specific input).
+// It is satisfied by errors.ErrUnsupported as well as some syscall errors.
+func IsNotSupported(err error) bool {
+	return errors.Is(err, errors.ErrUnsupported)
+}

internal/lockedfile/internal/filelock/filelock_fcntl.go

@@ -0,0 +1,210 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build aix || (solaris && !illumos)
+
+// This code implements the filelock API using POSIX 'fcntl' locks, which attach
+// to an (inode, process) pair rather than a file descriptor. To avoid unlocking
+// files prematurely when the same file is opened through different descriptors,
+// we allow only one read-lock at a time.
+//
+// Most platforms provide some alternative API, such as an 'flock' system call
+// or an F_OFD_SETLK command for 'fcntl', that allows for better concurrency and
+// does not require per-inode bookkeeping in the application.
+
+package filelock
+
+import (
+	"errors"
+	"io"
+	"io/fs"
+	"math/rand"
+	"sync"
+	"syscall"
+	"time"
+)
+
+type lockType int16
+
+const (
+	readLock  lockType = syscall.F_RDLCK
+	writeLock lockType = syscall.F_WRLCK
+)
+
+type inode = uint64 // type of syscall.Stat_t.Ino
+
+type inodeLock struct {
+	owner File
+	queue []<-chan File
+}
+
+var (
+	mu     sync.Mutex
+	inodes = map[File]inode{}
+	locks  = map[inode]inodeLock{}
+)
+
+func lock(f File, lt lockType) (err error) {
+	// POSIX locks apply per inode and process, and the lock for an inode is
+	// released when *any* descriptor for that inode is closed. So we need to
+	// synchronize access to each inode internally, and must serialize lock and
+	// unlock calls that refer to the same inode through different descriptors.
+	fi, err := f.Stat()
+	if err != nil {
+		return err
+	}
+	ino := fi.Sys().(*syscall.Stat_t).Ino
+
+	mu.Lock()
+	if i, dup := inodes[f]; dup && i != ino {
+		mu.Unlock()
+		return &fs.PathError{
+			Op:   lt.String(),
+			Path: f.Name(),
+			Err:  errors.New("inode for file changed since last Lock or RLock"),
+		}
+	}
+	inodes[f] = ino
+
+	var wait chan File
+	l := locks[ino]
+	if l.owner == f {
+		// This file already owns the lock, but the call may change its lock type.
+	} else if l.owner == nil {
+		// No owner: it's ours now.
+		l.owner = f
+	} else {
+		// Already owned: add a channel to wait on.
+		wait = make(chan File)
+		l.queue = append(l.queue, wait)
+	}
+	locks[ino] = l
+	mu.Unlock()
+
+	if wait != nil {
+		wait <- f
+	}
+
+	// Spurious EDEADLK errors arise on platforms that compute deadlock graphs at
+	// the process, rather than thread, level. Consider processes P and Q, with
+	// threads P.1, P.2, and Q.3. The following trace is NOT a deadlock, but will be
+	// reported as a deadlock on systems that consider only process granularity:
+	//
+	// 	P.1 locks file A.
+	// 	Q.3 locks file B.
+	// 	Q.3 blocks on file A.
+	// 	P.2 blocks on file B. (This is erroneously reported as a deadlock.)
+	// 	P.1 unlocks file A.
+	// 	Q.3 unblocks and locks file A.
+	// 	Q.3 unlocks files A and B.
+	// 	P.2 unblocks and locks file B.
+	// 	P.2 unlocks file B.
+	//
+	// These spurious errors were observed in practice on AIX and Solaris in
+	// cmd/go: see https://golang.org/issue/32817.
+	//
+	// We work around this bug by treating EDEADLK as always spurious. If there
+	// really is a lock-ordering bug between the interacting processes, it will
+	// become a livelock instead, but that's not appreciably worse than if we had
+	// a proper flock implementation (which generally does not even attempt to
+	// diagnose deadlocks).
+	//
+	// In the above example, that changes the trace to:
+	//
+	// 	P.1 locks file A.
+	// 	Q.3 locks file B.
+	// 	Q.3 blocks on file A.
+	// 	P.2 spuriously fails to lock file B and goes to sleep.
+	// 	P.1 unlocks file A.
+	// 	Q.3 unblocks and locks file A.
+	// 	Q.3 unlocks files A and B.
+	// 	P.2 wakes up and locks file B.
+	// 	P.2 unlocks file B.
+	//
+	// We know that the retry loop will not introduce a *spurious* livelock
+	// because, according to the POSIX specification, EDEADLK is only to be
+	// returned when “the lock is blocked by a lock from another process”.
+	// If that process is blocked on some lock that we are holding, then the
+	// resulting livelock is due to a real deadlock (and would manifest as such
+	// when using, for example, the flock implementation of this package).
+	// If the other process is *not* blocked on some other lock that we are
+	// holding, then it will eventually release the requested lock.
+
+	nextSleep := 1 * time.Millisecond
+	const maxSleep = 500 * time.Millisecond
+	for {
+		err = setlkw(f.Fd(), lt)
+		if err != syscall.EDEADLK {
+			break
+		}
+		time.Sleep(nextSleep)
+
+		nextSleep += nextSleep
+		if nextSleep > maxSleep {
+			nextSleep = maxSleep
+		}
+		// Apply 10% jitter to avoid synchronizing collisions when we finally unblock.
+		nextSleep += time.Duration((0.1*rand.Float64() - 0.05) * float64(nextSleep))
+	}
+
+	if err != nil {
+		unlock(f)
+		return &fs.PathError{
+			Op:   lt.String(),
+			Path: f.Name(),
+			Err:  err,
+		}
+	}
+
+	return nil
+}
+
+func unlock(f File) error {
+	var owner File
+
+	mu.Lock()
+	ino, ok := inodes[f]
+	if ok {
+		owner = locks[ino].owner
+	}
+	mu.Unlock()
+
+	if owner != f {
+		panic("unlock called on a file that is not locked")
+	}
+
+	err := setlkw(f.Fd(), syscall.F_UNLCK)
+
+	mu.Lock()
+	l := locks[ino]
+	if len(l.queue) == 0 {
+		// No waiters: remove the map entry.
+		delete(locks, ino)
+	} else {
+		// The first waiter is sending us their file now.
+		// Receive it and update the queue.
+		l.owner = <-l.queue[0]
+		l.queue = l.queue[1:]
+		locks[ino] = l
+	}
+	delete(inodes, f)
+	mu.Unlock()
+
+	return err
+}
+
+// setlkw calls FcntlFlock with F_SETLKW for the entire file indicated by fd.
+func setlkw(fd uintptr, lt lockType) error {
+	for {
+		err := syscall.FcntlFlock(fd, syscall.F_SETLKW, &syscall.Flock_t{
+			Type:   int16(lt),
+			Whence: io.SeekStart,
+			Start:  0,
+			Len:    0, // All bytes.
+		})
+		if err != syscall.EINTR {
+			return err
+		}
+	}
+}

internal/lockedfile/internal/filelock/filelock_other.go

@@ -0,0 +1,35 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build !unix && !windows
+
+package filelock
+
+import (
+	"errors"
+	"io/fs"
+)
+
+type lockType int8
+
+const (
+	readLock = iota + 1
+	writeLock
+)
+
+func lock(f File, lt lockType) error {
+	return &fs.PathError{
+		Op:   lt.String(),
+		Path: f.Name(),
+		Err:  errors.ErrUnsupported,
+	}
+}
+
+func unlock(f File) error {
+	return &fs.PathError{
+		Op:   "Unlock",
+		Path: f.Name(),
+		Err:  errors.ErrUnsupported,
+	}
+}

internal/lockedfile/internal/filelock/filelock_test.go

@@ -0,0 +1,209 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build !js && !plan9 && !wasip1
+
+package filelock_test
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+	"time"
+
+	"hakurei.app/container"
+	"hakurei.app/internal/lockedfile/internal/filelock"
+	"hakurei.app/internal/lockedfile/internal/testexec"
+)
+
+func lock(t *testing.T, f *os.File) {
+	t.Helper()
+	err := filelock.Lock(f)
+	t.Logf("Lock(fd %d) = %v", f.Fd(), err)
+	if err != nil {
+		t.Fail()
+	}
+}
+
+func rLock(t *testing.T, f *os.File) {
+	t.Helper()
+	err := filelock.RLock(f)
+	t.Logf("RLock(fd %d) = %v", f.Fd(), err)
+	if err != nil {
+		t.Fail()
+	}
+}
+
+func unlock(t *testing.T, f *os.File) {
+	t.Helper()
+	err := filelock.Unlock(f)
+	t.Logf("Unlock(fd %d) = %v", f.Fd(), err)
+	if err != nil {
+		t.Fail()
+	}
+}
+
+func mustTempFile(t *testing.T) (f *os.File, remove func()) {
+	t.Helper()
+
+	base := filepath.Base(t.Name())
+	f, err := os.CreateTemp("", base)
+	if err != nil {
+		t.Fatalf(`os.CreateTemp("", %q) = %v`, base, err)
+	}
+	t.Logf("fd %d = %s", f.Fd(), f.Name())
+
+	return f, func() {
+		f.Close()
+		os.Remove(f.Name())
+	}
+}
+
+func mustOpen(t *testing.T, name string) *os.File {
+	t.Helper()
+
+	f, err := os.OpenFile(name, os.O_RDWR, 0)
+	if err != nil {
+		t.Fatalf("os.OpenFile(%q) = %v", name, err)
+	}
+
+	t.Logf("fd %d = os.OpenFile(%q)", f.Fd(), name)
+	return f
+}
+
+const (
+	quiescent            = 10 * time.Millisecond
+	probablyStillBlocked = 10 * time.Second
+)
+
+func mustBlock(t *testing.T, op string, f *os.File) (wait func(*testing.T)) {
+	t.Helper()
+
+	desc := fmt.Sprintf("%s(fd %d)", op, f.Fd())
+
+	done := make(chan struct{})
+	go func() {
+		t.Helper()
+		switch op {
+		case "Lock":
+			lock(t, f)
+		case "RLock":
+			rLock(t, f)
+		default:
+			panic("invalid op: " + op)
+		}
+		close(done)
+	}()
+
+	select {
+	case <-done:
+		t.Fatalf("%s unexpectedly did not block", desc)
+		return nil
+
+	case <-time.After(quiescent):
+		t.Logf("%s is blocked (as expected)", desc)
+		return func(t *testing.T) {
+			t.Helper()
+			select {
+			case <-time.After(probablyStillBlocked):
+				t.Fatalf("%s is unexpectedly still blocked", desc)
+			case <-done:
+			}
+		}
+	}
+}
+
+func TestLockExcludesLock(t *testing.T) {
+	t.Parallel()
+
+	f, remove := mustTempFile(t)
+	defer remove()
+
+	other := mustOpen(t, f.Name())
+	defer other.Close()
+
+	lock(t, f)
+	lockOther := mustBlock(t, "Lock", other)
+	unlock(t, f)
+	lockOther(t)
+	unlock(t, other)
+}
+
+func TestLockExcludesRLock(t *testing.T) {
+	t.Parallel()
+
+	f, remove := mustTempFile(t)
+	defer remove()
+
+	other := mustOpen(t, f.Name())
+	defer other.Close()
+
+	lock(t, f)
+	rLockOther := mustBlock(t, "RLock", other)
+	unlock(t, f)
+	rLockOther(t)
+	unlock(t, other)
+}
+
+func TestRLockExcludesOnlyLock(t *testing.T) {
+	t.Parallel()
+
+	f, remove := mustTempFile(t)
+	defer remove()
+	rLock(t, f)
+
+	f2 := mustOpen(t, f.Name())
+	defer f2.Close()
+
+	doUnlockTF := false
+	switch runtime.GOOS {
+	case "aix", "solaris":
+		// When using POSIX locks (as on Solaris), we can't safely read-lock the
+		// same inode through two different descriptors at the same time: when the
+		// first descriptor is closed, the second descriptor would still be open but
+		// silently unlocked. So a second RLock must block instead of proceeding.
+		lockF2 := mustBlock(t, "RLock", f2)
+		unlock(t, f)
+		lockF2(t)
+	default:
+		rLock(t, f2)
+		doUnlockTF = true
+	}
+
+	other := mustOpen(t, f.Name())
+	defer other.Close()
+	lockOther := mustBlock(t, "Lock", other)
+
+	unlock(t, f2)
+	if doUnlockTF {
+		unlock(t, f)
+	}
+	lockOther(t)
+	unlock(t, other)
+}
+
+func TestLockNotDroppedByExecCommand(t *testing.T) {
+	f, remove := mustTempFile(t)
+	defer remove()
+
+	lock(t, f)
+
+	other := mustOpen(t, f.Name())
+	defer other.Close()
+
+	// Some kinds of file locks are dropped when a duplicated or forked file
+	// descriptor is unlocked. Double-check that the approach used by os/exec does
+	// not accidentally drop locks.
+	cmd := testexec.CommandContext(t, t.Context(), container.MustExecutable(nil), "-test.run=^$")
+	if err := cmd.Run(); err != nil {
+		t.Fatalf("exec failed: %v", err)
+	}
+
+	lockOther := mustBlock(t, "Lock", other)
+	unlock(t, f)
+	lockOther(t)
+	unlock(t, other)
+}

internal/lockedfile/internal/filelock/filelock_unix.go

@@ -0,0 +1,40 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build darwin || dragonfly || freebsd || illumos || linux || netbsd || openbsd
+
+package filelock
+
+import (
+	"io/fs"
+	"syscall"
+)
+
+type lockType int16
+
+const (
+	readLock  lockType = syscall.LOCK_SH
+	writeLock lockType = syscall.LOCK_EX
+)
+
+func lock(f File, lt lockType) (err error) {
+	for {
+		err = syscall.Flock(int(f.Fd()), int(lt))
+		if err != syscall.EINTR {
+			break
+		}
+	}
+	if err != nil {
+		return &fs.PathError{
+			Op:   lt.String(),
+			Path: f.Name(),
+			Err:  err,
+		}
+	}
+	return nil
+}
+
+func unlock(f File) error {
+	return lock(f, syscall.LOCK_UN)
+}

internal/lockedfile/internal/filelock/filelock_windows.go

@@ -0,0 +1,57 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build windows
+
+package filelock
+
+import (
+	"internal/syscall/windows"
+	"io/fs"
+	"syscall"
+)
+
+type lockType uint32
+
+const (
+	readLock  lockType = 0
+	writeLock lockType = windows.LOCKFILE_EXCLUSIVE_LOCK
+)
+
+const (
+	reserved = 0
+	allBytes = ^uint32(0)
+)
+
+func lock(f File, lt lockType) error {
+	// Per https://golang.org/issue/19098, “Programs currently expect the Fd
+	// method to return a handle that uses ordinary synchronous I/O.”
+	// However, LockFileEx still requires an OVERLAPPED structure,
+	// which contains the file offset of the beginning of the lock range.
+	// We want to lock the entire file, so we leave the offset as zero.
+	ol := new(syscall.Overlapped)
+
+	err := windows.LockFileEx(syscall.Handle(f.Fd()), uint32(lt), reserved, allBytes, allBytes, ol)
+	if err != nil {
+		return &fs.PathError{
+			Op:   lt.String(),
+			Path: f.Name(),
+			Err:  err,
+		}
+	}
+	return nil
+}
+
+func unlock(f File) error {
+	ol := new(syscall.Overlapped)
+	err := windows.UnlockFileEx(syscall.Handle(f.Fd()), reserved, allBytes, allBytes, ol)
+	if err != nil {
+		return &fs.PathError{
+			Op:   "Unlock",
+			Path: f.Name(),
+			Err:  err,
+		}
+	}
+	return nil
+}

internal/lockedfile/internal/testexec/exec.go

@@ -0,0 +1,43 @@
+package testexec
+
+import (
+	"context"
+	"os/exec"
+	"syscall"
+	"testing"
+)
+
+// CommandContext is like exec.CommandContext, but:
+//   - sends SIGQUIT instead of SIGKILL in its Cancel function
+//   - fails the test if the command does not complete before the context is canceled, and
+//   - sets a Cleanup function that verifies that the test did not leak a subprocess.
+func CommandContext(t testing.TB, ctx context.Context, name string, args ...string) *exec.Cmd {
+	t.Helper()
+
+	cmd := exec.CommandContext(ctx, name, args...)
+	cmd.Cancel = func() error {
+		if ctx.Err() == context.DeadlineExceeded {
+			// The command timed out due to running too close to the test's deadline.
+			// There is no way the test did that intentionally — it's too close to the
+			// wire! — so mark it as a test failure. That way, if the test expects the
+			// command to fail for some other reason, it doesn't have to distinguish
+			// between that reason and a timeout.
+			t.Errorf("test timed out while running command: %v", cmd)
+		} else {
+			// The command is being terminated due to ctx being canceled, but
+			// apparently not due to an explicit test deadline that we added.
+			// Log that information in case it is useful for diagnosing a failure,
+			// but don't actually fail the test because of it.
+			t.Logf("%v: terminating command: %v", ctx.Err(), cmd)
+		}
+		return cmd.Process.Signal(syscall.SIGQUIT)
+	}
+
+	t.Cleanup(func() {
+		if cmd.Process != nil && cmd.ProcessState == nil {
+			t.Errorf("command was started, but test did not wait for it to complete: %v", cmd)
+		}
+	})
+
+	return cmd
+}