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>

4 files changed, 52 insertions, 124 deletions

diff --git a/internal/app/process.go b/internal/app/process.go
index 8e18ae64..a0950c27 100644
--- a/internal/app/process.go
+++ b/internal/app/process.go
@@ -192,12 +192,6 @@ func (ms mainState) beforeExit(isFault bool) {
 	} else if ms.uintptr&mainNeedsDestroy != 0 {
 		panic("unreachable")
 	}
-
-	if ms.store != nil {
-		if err := ms.store.Close(); err != nil {
-			perror(err, "close state store")
-		}
-	}
 }
 
 // fatal calls printMessageError, performs necessary cleanup, followed by a call to [os.Exit](1).
diff --git a/internal/app/state/multi.go b/internal/app/state/multi.go
index c9096ace..9824a8ee 100644
--- a/internal/app/state/multi.go
+++ b/internal/app/state/multi.go
@@ -9,12 +9,15 @@ import (
 	"path"
 	"strconv"
 	"sync"
-	"syscall"
 
 	"hakurei.app/hst"
+	"hakurei.app/internal/lockedfile"
 	"hakurei.app/message"
 )
 
+// multiLockFileName is the name of the file backing [lockedfile.Mutex] of a multiBackend.
+const multiLockFileName = "lock"
+
 // fine-grained locking and access
 type multiStore struct {
 	base string
@@ -44,19 +47,18 @@ func (s *multiStore) Do(identity int, f func(c Cursor)) (bool, error) {
 			return false, &hst.AppError{Step: "create store segment directory", Err: err}
 		}
 
-		// open locker file
-		if l, err := os.OpenFile(b.path+".lock", os.O_RDWR|os.O_CREATE, 0600); err != nil {
-			s.backends.CompareAndDelete(identity, b)
-			return false, &hst.AppError{Step: "open store segment lock file", Err: err}
-		} else {
-			b.lockfile = l
-		}
+		// set up file-based mutex
+		b.lockfile = lockedfile.MutexAt(path.Join(b.path, multiLockFileName))
+
 		b.mu.Unlock()
 	}
 
 	// lock backend
-	if err := b.lockFile(); err != nil {
+	if unlock, err := b.lockfile.Lock(); err != nil {
 		return false, &hst.AppError{Step: "lock store segment", Err: err}
+	} else {
+		// unlock backend after Do is complete
+		defer unlock()
 	}
 
 	// expose backend methods without exporting the pointer
@@ -66,10 +68,6 @@ func (s *multiStore) Do(identity int, f func(c Cursor)) (bool, error) {
 	// disable access to the backend on a best-effort basis
 	c.multiBackend = nil
 
-	// unlock backend
-	if err := b.unlockFile(); err != nil {
-		return true, &hst.AppError{Step: "unlock store segment", Err: err}
-	}
 	return true, nil
 }
 
@@ -108,59 +106,17 @@ func (s *multiStore) List() ([]int, error) {
 	return append([]int(nil), aidsBuf...), nil
 }
 
-func (s *multiStore) Close() error {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	var errs []error
-	s.backends.Range(func(_, value any) bool {
-		b := value.(*multiBackend)
-		errs = append(errs, b.close())
-		return true
-	})
-
-	return errors.Join(errs...)
-}
-
 type multiBackend struct {
 	path string
 
 	// created/opened by prepare
-	lockfile *os.File
+	lockfile *lockedfile.Mutex
 
 	mu sync.RWMutex
 }
 
 func (b *multiBackend) filename(id *hst.ID) string { return path.Join(b.path, id.String()) }
 
-func (b *multiBackend) lockFileAct(lt int) (err error) {
-	op := "LockAct"
-	switch lt {
-	case syscall.LOCK_EX:
-		op = "Lock"
-	case syscall.LOCK_UN:
-		op = "Unlock"
-	}
-
-	for {
-		err = syscall.Flock(int(b.lockfile.Fd()), lt)
-		if !errors.Is(err, syscall.EINTR) {
-			break
-		}
-	}
-	if err != nil {
-		return &fs.PathError{
-			Op:   op,
-			Path: b.lockfile.Name(),
-			Err:  err,
-		}
-	}
-	return nil
-}
-
-func (b *multiBackend) lockFile() error   { return b.lockFileAct(syscall.LOCK_EX) }
-func (b *multiBackend) unlockFile() error { return b.lockFileAct(syscall.LOCK_UN) }
-
 // reads all launchers in simpleBackend
 // file contents are ignored if decode is false
 func (b *multiBackend) load(decode bool) (map[hst.ID]*hst.State, error) {
@@ -184,6 +140,11 @@ func (b *multiBackend) load(decode bool) (map[hst.ID]*hst.State, error) {
 			return nil, fmt.Errorf("unexpected directory %q in store", e.Name())
 		}
 
+		// skip lock file
+		if e.Name() == multiLockFileName {
+			continue
+		}
+
 		var id hst.ID
 		if err := id.UnmarshalText([]byte(e.Name())); err != nil {
 			return nil, &hst.AppError{Step: "parse state key", Err: err}
@@ -268,17 +229,6 @@ func (b *multiBackend) Len() (int, error) {
 	return len(rn), nil
 }
 
-func (b *multiBackend) close() error {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-
-	err := b.lockfile.Close()
-	if err == nil || errors.Is(err, os.ErrInvalid) || errors.Is(err, os.ErrClosed) {
-		return nil
-	}
-	return &hst.AppError{Step: "close lock file", Err: err}
-}
-
 // NewMulti returns an instance of the multi-file store.
 func NewMulti(msg message.Msg, runDir string) Store {
 	return &multiStore{
diff --git a/internal/app/state/state.go b/internal/app/state/state.go
index ef783bfb..c5238d18 100644
--- a/internal/app/state/state.go
+++ b/internal/app/state/state.go
@@ -19,9 +19,6 @@ type Store interface {
 	// List queries the store and returns a list of identities known to the store.
 	// Note that some or all returned identities might not have any active apps.
 	List() (identities []int, err error)
-
-	// Close releases any resources held by Store.
-	Close() error
 }
 
 // Cursor provides access to the store of an identity.
diff --git a/internal/app/state/state_test.go b/internal/app/state/state_test.go
index 2e191f31..a334c348 100644
--- a/internal/app/state/state_test.go
+++ b/internal/app/state/state_test.go
@@ -62,62 +62,49 @@ func testStore(t *testing.T, s state.Store) {
 		})
 	}
 
-	t.Run("insert entry checked", func(t *testing.T) {
-		insert(insertEntryChecked, 0)
-		check(insertEntryChecked, 0)
-	})
+	// insert entry checked
+	insert(insertEntryChecked, 0)
+	check(insertEntryChecked, 0)
 
-	t.Run("insert entry unchecked", func(t *testing.T) {
-		insert(insertEntryNoCheck, 0)
-	})
+	// insert entry unchecked
+	insert(insertEntryNoCheck, 0)
 
-	t.Run("insert entry different identity", func(t *testing.T) {
-		insert(insertEntryOtherApp, 1)
-		check(insertEntryOtherApp, 1)
-	})
+	// insert entry different identity
+	insert(insertEntryOtherApp, 1)
+	check(insertEntryOtherApp, 1)
 
-	t.Run("check previous insertion", func(t *testing.T) {
-		check(insertEntryNoCheck, 0)
-	})
+	// check previous insertion
+	check(insertEntryNoCheck, 0)
 
-	t.Run("list identities", func(t *testing.T) {
-		if identities, err := s.List(); err != nil {
-			t.Fatalf("List: error = %v", err)
-		} else {
-			slices.Sort(identities)
-			want := []int{0, 1}
-			if !slices.Equal(identities, want) {
-				t.Fatalf("List() = %#v, want %#v", identities, want)
-			}
+	// list identities
+	if identities, err := s.List(); err != nil {
+		t.Fatalf("List: error = %v", err)
+	} else {
+		slices.Sort(identities)
+		want := []int{0, 1}
+		if !slices.Equal(identities, want) {
+			t.Fatalf("List() = %#v, want %#v", identities, want)
 		}
-	})
+	}
 
-	t.Run("join store", func(t *testing.T) {
-		if entries, err := state.Join(s); err != nil {
-			t.Fatalf("Join: error = %v", err)
-		} else if len(entries) != 3 {
-			t.Fatalf("Join(s) = %#v", entries)
-		}
-	})
+	// join store
+	if entries, err := state.Join(s); err != nil {
+		t.Fatalf("Join: error = %v", err)
+	} else if len(entries) != 3 {
+		t.Fatalf("Join(s) = %#v", entries)
+	}
 
-	t.Run("clear identity 1", func(t *testing.T) {
-		do(1, func(c state.Cursor) {
-			if err := c.Destroy(tc[insertEntryOtherApp].ID); err != nil {
-				t.Fatalf("Destroy: error = %v", err)
-			}
-		})
-		do(1, func(c state.Cursor) {
-			if l, err := c.Len(); err != nil {
-				t.Fatalf("Len: error = %v", err)
-			} else if l != 0 {
-				t.Fatalf("Len: %d, want 0", l)
-			}
-		})
+	// clear identity 1
+	do(1, func(c state.Cursor) {
+		if err := c.Destroy(tc[insertEntryOtherApp].ID); err != nil {
+			t.Fatalf("Destroy: error = %v", err)
+		}
 	})
-
-	t.Run("close store", func(t *testing.T) {
-		if err := s.Close(); err != nil {
-			t.Fatalf("Close: error = %v", err)
+	do(1, func(c state.Cursor) {
+		if l, err := c.Len(); err != nil {
+			t.Fatalf("Len: error = %v", err)
+		} else if l != 0 {
+			t.Fatalf("Len: %d, want 0", l)
 		}
 	})
 }