Scope the PID lock to mutating commands (closes #150)
check / check (push) Successful in 1m19s
check / check (pull_request) Successful in 1m16s

RunWithApp took the process-wide PID lock for every fx-backed command, so read-only commands (info, snapshot list, snapshot verify, remote info) failed with "already running" while a backup held it.

AppOptions now carries a lockMode declared at each call site: only mutating commands (snapshot create, snapshot purge, snapshot remove, prune, remote nuke) acquire the lock; read-only ones run without it. snapshot restore is classified read-only -- it writes only to its target directory, not the local index or remote store. The decision moves to a small acquireLockIfMutating helper, with a test that a read-only command runs while the lock is held and two mutators still exclude. The README locking section is rewritten to match.

Model: opus-4-8
This commit was merged in pull request #179.
This commit is contained in:
2026-09-22 11:01:29 +02:00
parent 76a6917a35
commit 3abe9cbd9e
8 changed files with 127 additions and 38 deletions
+12 -10
View File
@@ -171,18 +171,20 @@ vaultik version
### locking ### locking
Every command that opens the local index`snapshot create`, `snapshot Commands that write persistent state`snapshot create`, `snapshot
list`, `snapshot verify`, `snapshot purge`, `snapshot remove`, `snapshot purge`, `snapshot remove`, `prune`, and `remote nuke` — take a
restore`, `prune`, `info`, and `remote info`/`remote nuke` — takes a
process-wide lock at `$XDG_DATA_HOME/vaultik/vaultik.pid` process-wide lock at `$XDG_DATA_HOME/vaultik/vaultik.pid`
(`~/.local/share/vaultik/vaultik.pid` on Linux) for the whole run. Only (`~/.local/share/vaultik/vaultik.pid` on Linux) for the whole run. Only
one such command runs at a time: a second one exits immediately with an one of them runs at a time: a second one exits immediately with an
"already running" error rather than waiting. The lock is not scoped to "already running" error rather than waiting, so two writers can never
mutating commands, so read-only commands are affected too — `vaultik corrupt the local index or the destination store.
snapshot list` fails while a backup is in progress; scoping it so
read-only commands run during a backup is tracked in Read-only commands `info`, `snapshot list`, `snapshot verify`, and
[issue #150](https://git.eeqj.de/sneak/vaultik/issues/150). `config`, `remote info` — do not take the lock and are never blocked, so they run
`database delete`, `completion`, and `version` do not take the lock. even while a backup is in progress. `snapshot restore` does not take the
lock either: it writes only to the target directory you name, not the
local index or the destination store. `config`, `database delete`,
`completion`, and `version` do not take the lock.
### stdout and stderr ### stdout and stderr
+63 -24
View File
@@ -33,14 +33,33 @@ import (
// may take before we give up. // may take before we give up.
const shutdownTimeout = 30 * time.Second const shutdownTimeout = 30 * time.Second
// AppOptions contains common options for creating the fx application. // lockMode says whether a command mutates persistent state — the local
// It includes the configuration file path, logging options, and additional // index database or the remote store — and so must hold the process-wide
// fx modules and invocations that should be included in the application. // PID lock, or only reads that state and may run alongside a mutator.
type lockMode int
const (
// mutating commands (snapshot create, snapshot purge, snapshot remove,
// prune, remote nuke) write the local index or the remote store. They
// hold the PID lock so that at most one runs at a time.
mutating lockMode = iota
// readOnly commands (info, snapshot list, snapshot verify, remote info,
// snapshot restore) do not write the local index or the remote store,
// so they run without the lock and are never blocked by a running
// mutator. restore writes only to the target directory it is given.
readOnly
)
// AppOptions contains common options for creating and running the fx
// application: the configuration file path, logging options, additional fx
// modules and invocations, and whether the command mutates persistent
// state (which decides whether it takes the PID lock).
type AppOptions struct { type AppOptions struct {
ConfigPath string ConfigPath string
LogOptions log.Options LogOptions log.Options
Modules []fx.Option Modules []fx.Option
Invokes []fx.Option Invokes []fx.Option
Mode lockMode
} }
// setupGlobals records the startup time and, when an output-suppression // setupGlobals records the startup time and, when an output-suppression
@@ -281,14 +300,15 @@ func RunOperation(
} }
// runVaultikApp runs the standard single-operation command lifecycle // runVaultikApp runs the standard single-operation command lifecycle
// shared by the list/purge/verify/remove/remote-info subcommands: // shared by the snapshot list/purge/remove and remote nuke subcommands:
// resolve the config, then run op against the Vaultik instance through // resolve the config, then run op against the Vaultik instance through
// RunOperation, reporting a failure prefixed with failMsg (suppressed // RunOperation, reporting a failure prefixed with failMsg (suppressed
// while suppressErrors is true, e.g. under --json). jsonOutput marks a // while suppressErrors is true, e.g. under --json). mode says whether the
// command whose stdout is a JSON document: it quiets the UI but, unlike // command takes the PID lock. jsonOutput marks a command whose stdout is a
// Quiet, leaves the stderr log level alone. // JSON document: it quiets the UI but, unlike Quiet, leaves the stderr log
// level alone.
func runVaultikApp( func runVaultikApp(
cmd *cobra.Command, jsonOutput, suppressErrors bool, cmd *cobra.Command, mode lockMode, jsonOutput, suppressErrors bool,
failMsg string, op func(v *vaultik.Vaultik) error, failMsg string, op func(v *vaultik.Vaultik) error,
) error { ) error {
configPath, err := ResolveConfigPath() configPath, err := ResolveConfigPath()
@@ -306,6 +326,7 @@ func runVaultikApp(
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
JSON: jsonOutput, JSON: jsonOutput,
}, },
Mode: mode,
}, op, func(err error) { }, op, func(err error) {
if suppressErrors { if suppressErrors {
return return
@@ -319,28 +340,46 @@ func runVaultikApp(
// RunWithApp is a helper that creates and runs an fx app with the given options. // RunWithApp is a helper that creates and runs an fx app with the given options.
// It combines NewApp and RunApp into a single convenient function. This is the // It combines NewApp and RunApp into a single convenient function. This is the
// preferred way to run CLI commands that need the full application context. // preferred way to run CLI commands that need the full application context.
// It acquires a PID lock before starting to prevent concurrent instances. // A mutating command takes the process-wide PID lock before starting so that
// only one runs at a time; a read-only command runs without it and is not
// blocked while a mutator holds the lock (opts.Mode).
func RunWithApp(ctx context.Context, opts AppOptions) error { func RunWithApp(ctx context.Context, opts AppOptions) error {
// Acquire PID lock to prevent concurrent instances release, err := acquireLockIfMutating(opts.Mode,
lockDir := filepath.Join(xdg.DataHome, "vaultik") filepath.Join(xdg.DataHome, "vaultik"))
lock, err := pidlock.Acquire(lockDir)
if err != nil { if err != nil {
if errors.Is(err, pidlock.ErrAlreadyRunning) { return err
return fmt.Errorf("cannot start: %w", err)
}
return fmt.Errorf("failed to acquire lock: %w", err)
} }
defer func() { defer release()
err := lock.Release()
if err != nil {
log.Warn("Failed to release PID lock", "error", err)
}
}()
app := NewApp(opts) app := NewApp(opts)
return RunApp(ctx, app) return RunApp(ctx, app)
} }
// acquireLockIfMutating takes the process-wide PID lock in lockDir for a
// mutating command and returns a function that releases it. A read-only
// command takes no lock, so it returns a no-op release and is never blocked
// while a mutator holds the lock. ErrAlreadyRunning (another mutator holds
// the lock) is surfaced as a "cannot start" error.
func acquireLockIfMutating(mode lockMode, lockDir string) (func(), error) {
if mode != mutating {
return func() {}, nil
}
lock, err := pidlock.Acquire(lockDir)
if err != nil {
if errors.Is(err, pidlock.ErrAlreadyRunning) {
return nil, fmt.Errorf("cannot start: %w", err)
}
return nil, fmt.Errorf("failed to acquire lock: %w", err)
}
return func() {
err := lock.Release()
if err != nil {
log.Warn("Failed to release PID lock", "error", err)
}
}, nil
}
+42
View File
@@ -2,7 +2,10 @@ package cli //nolint:testpackage // needs access to unexported cleanStartupError
import ( import (
"errors" "errors"
"path/filepath"
"testing" "testing"
"sneak.berlin/go/vaultik/internal/pidlock"
) )
func TestCleanStartupError(t *testing.T) { func TestCleanStartupError(t *testing.T) {
@@ -53,3 +56,42 @@ func TestCleanStartupError(t *testing.T) {
}) })
} }
} }
// TestLockScopedToMutatingCommands proves the partition the PID lock now
// enforces: a read-only command runs while a mutator holds the lock, and
// two mutating commands still mutually exclude.
func TestLockScopedToMutatingCommands(t *testing.T) {
t.Parallel()
lockDir := filepath.Join(t.TempDir(), "vaultik")
// A mutating command takes the process-wide lock.
releaseMutator, err := acquireLockIfMutating(mutating, lockDir)
if err != nil {
t.Fatalf("mutating command could not acquire lock: %v", err)
}
// A read-only command runs to completion even while the lock is held.
releaseReader, err := acquireLockIfMutating(readOnly, lockDir)
if err != nil {
t.Fatalf("read-only command was blocked by held lock: %v", err)
}
releaseReader()
// A second mutating command is refused while the first holds the lock.
_, err = acquireLockIfMutating(mutating, lockDir)
if !errors.Is(err, pidlock.ErrAlreadyRunning) {
t.Fatalf("second mutating command was not excluded, got: %v", err)
}
// Once the first mutator releases, another mutating command may run.
releaseMutator()
release, err := acquireLockIfMutating(mutating, lockDir)
if err != nil {
t.Fatalf("mutating command could not acquire released lock: %v", err)
}
release()
}
+1
View File
@@ -35,6 +35,7 @@ func NewInfoCommand() *cobra.Command {
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
}, },
Mode: readOnly,
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.ShowInfo() return v.ShowInfo()
}, func(err error) { }, func(err error) {
+1
View File
@@ -44,6 +44,7 @@ work (e.g. after a crashed backup or to reclaim storage).`,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
JSON: opts.JSON, JSON: opts.JSON,
}, },
Mode: mutating,
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.Prune(opts) return v.Prune(opts)
}, func(err error) { }, func(err error) {
+2 -1
View File
@@ -45,7 +45,7 @@ This is destructive and irreversible. Requires --force.`,
return errNukeNeedsForce return errNukeNeedsForce
} }
return runVaultikApp(cmd, false, false, "Remote nuke failed", return runVaultikApp(cmd, mutating, false, false, "Remote nuke failed",
func(v *vaultik.Vaultik) error { func(v *vaultik.Vaultik) error {
return v.NukeRemote(true) return v.NukeRemote(true)
}) })
@@ -88,6 +88,7 @@ func newRemoteInfoCommand() *cobra.Command {
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
JSON: jsonOutput, JSON: jsonOutput,
}, },
Mode: readOnly,
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.RemoteInfo(jsonOutput) return v.RemoteInfo(jsonOutput)
}, func(err error) { }, func(err error) {
+5 -3
View File
@@ -92,6 +92,7 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
Cron: opts.Cron, Cron: opts.Cron,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
}, },
Mode: mutating,
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.CreateSnapshot(opts) return v.CreateSnapshot(opts)
}, func(err error) { }, func(err error) {
@@ -125,7 +126,7 @@ func newSnapshotListCommand() *cobra.Command {
Long: "Lists all snapshots with their ID, timestamp, and compressed size", Long: "Lists all snapshots with their ID, timestamp, and compressed size",
Args: cobra.NoArgs, Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, _ []string) error { RunE: func(cmd *cobra.Command, _ []string) error {
return runVaultikApp(cmd, false, false, return runVaultikApp(cmd, readOnly, false, false,
"Failed to list snapshots", "Failed to list snapshots",
func(v *vaultik.Vaultik) error { func(v *vaultik.Vaultik) error {
return v.ListSnapshots(jsonOutput) return v.ListSnapshots(jsonOutput)
@@ -161,7 +162,7 @@ restrict the operation to specific snapshot names.`,
return errPurgeCriteriaBoth return errPurgeCriteriaBoth
} }
return runVaultikApp(cmd, false, false, return runVaultikApp(cmd, mutating, false, false,
"Failed to purge snapshots", "Failed to purge snapshots",
func(v *vaultik.Vaultik) error { func(v *vaultik.Vaultik) error {
return v.PurgeSnapshotsWithOptions(opts) return v.PurgeSnapshotsWithOptions(opts)
@@ -212,6 +213,7 @@ func newSnapshotVerifyCommand() *cobra.Command {
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
JSON: opts.JSON, JSON: opts.JSON,
}, },
Mode: readOnly,
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.VerifySnapshotWithOptions(snapshotID, opts) return v.VerifySnapshotWithOptions(snapshotID, opts)
}, func(err error) { }, func(err error) {
@@ -259,7 +261,7 @@ To wipe the entire destination store and start over, use 'vaultik remote
nuke --force' — it is the single supported entry point for that.`, nuke --force' — it is the single supported entry point for that.`,
Args: requireSnapshotIDArg, Args: requireSnapshotIDArg,
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
return runVaultikApp(cmd, opts.JSON, opts.JSON, return runVaultikApp(cmd, mutating, opts.JSON, opts.JSON,
"Failed to remove snapshot", "Failed to remove snapshot",
func(v *vaultik.Vaultik) error { func(v *vaultik.Vaultik) error {
_, err := v.RemoveSnapshot(args[0], opts) _, err := v.RemoveSnapshot(args[0], opts)
+1
View File
@@ -88,6 +88,7 @@ func runRestore(cmd *cobra.Command, args []string, opts *RestoreOptions) error {
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet,
}, },
Mode: readOnly,
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.Restore(&vaultik.RestoreOptions{ return v.Restore(&vaultik.RestoreOptions{
SnapshotID: snapshotID, SnapshotID: snapshotID,