feat(nunciod): single-instance advisory lock on the DB path #346

Merged
KofTwentyTwo merged 1 commits from story/ws-c3-single-instance-lock into dev 2026-07-31 20:55:14 +00:00
KofTwentyTwo commented 2026-07-31 20:53:49 +00:00 (Migrated from github.com)

Summary

  • Closes the WS-C3 gap: nothing previously stopped two nunciod processes from opening the same SQLite database concurrently; only an incidental gRPC port-bind collision offered any protection, and that only fired if both instances also picked the same port.
  • nunciod::lock::InstanceLock::acquire(&db_path) now takes an exclusive OS advisory lock on a sidecar file <db_path>.lock before the store is opened (wired in crates/nunciod/src/main.rs, right after the db path is resolved and before SelfHealingSyncOrchestrator::initialize_and_recover). If another live instance already holds it, startup fails fast with another nunciod instance is already running for this database at <path> and main returns Err, exiting non-zero without ever touching the store.
  • The lock guard (_instance_lock) is bound in main and held for the full process lifetime; it drops (and the OS releases the flock/LockFileEx lock) at the end of main, i.e. after the existing graceful-shutdown drain completes. If the process dies uncleanly instead, the OS releases the lock the moment the handle closes — no PID file, nothing that can go stale.
  • Dependency added: fs4 = "0.13" (workspace + nunciod). Justification: Rust's std has no portable whole-file advisory lock API; fs4 is a small, widely used wrapper over flock(2) on Unix and LockFileEx on Windows, which is exactly what this needs instead of a fragile PID-file scheme.

New module

  • crates/nunciod/src/lock.rs: InstanceLock (guard) + InstanceLockError (thiserror, AlreadyRunning/Io variants).

Test plan

  • second_acquire_for_same_path_fails_while_first_guard_is_held — acquiring twice for the same path while the first guard is alive fails with AlreadyRunning.
  • acquire_succeeds_again_after_the_first_guard_is_dropped — dropping the first guard lets a fresh acquire for the same path succeed.
  • error_message_names_the_database_path — error text is the actionable message asked for and includes the db path.
  • lock_file_is_created_beside_the_database_path — sidecar .lock file appears next to the db path.
  • cargo fmt --all -- --check, cargo check-all (clippy --all-targets --workspace -D warnings), cargo test-all all green locally (pre-commit hook ran the same three gates and passed). nunciod unit suite: 131 passed / 0 failed / 0 ignored; full workspace cargo test-all (unit + integration + E2E, all crates): also green, 0 ignored.

Notes / follow-ups

  • Encountered one pre-existing flaky test unrelated to this change: nuncio-cli's mail_and_folder_report_honest_errors_when_daemon_unreachable (binds an ephemeral port, drops it, then expects the connect to that now-free port to fail) failed once on a background port-reuse race and passed on every other run, including inside the pre-commit hook that gated this commit. Not something this PR should fix; flagging in case it's worth a tracking issue for hardening that test's port-reuse assumption.
## Summary - Closes the WS-C3 gap: nothing previously stopped two `nunciod` processes from opening the same SQLite database concurrently; only an incidental gRPC port-bind collision offered any protection, and that only fired if both instances also picked the same port. - `nunciod::lock::InstanceLock::acquire(&db_path)` now takes an exclusive OS advisory lock on a sidecar file `<db_path>.lock` **before** the store is opened (wired in `crates/nunciod/src/main.rs`, right after the db path is resolved and before `SelfHealingSyncOrchestrator::initialize_and_recover`). If another live instance already holds it, startup fails fast with `another nunciod instance is already running for this database at <path>` and `main` returns `Err`, exiting non-zero without ever touching the store. - The lock guard (`_instance_lock`) is bound in `main` and held for the full process lifetime; it drops (and the OS releases the `flock`/`LockFileEx` lock) at the end of `main`, i.e. after the existing graceful-shutdown drain completes. If the process dies uncleanly instead, the OS releases the lock the moment the handle closes — no PID file, nothing that can go stale. - **Dependency added:** `fs4 = "0.13"` (workspace + `nunciod`). Justification: Rust's std has no portable whole-file advisory lock API; `fs4` is a small, widely used wrapper over `flock(2)` on Unix and `LockFileEx` on Windows, which is exactly what this needs instead of a fragile PID-file scheme. ## New module - `crates/nunciod/src/lock.rs`: `InstanceLock` (guard) + `InstanceLockError` (`thiserror`, `AlreadyRunning`/`Io` variants). ## Test plan - [x] `second_acquire_for_same_path_fails_while_first_guard_is_held` — acquiring twice for the same path while the first guard is alive fails with `AlreadyRunning`. - [x] `acquire_succeeds_again_after_the_first_guard_is_dropped` — dropping the first guard lets a fresh acquire for the same path succeed. - [x] `error_message_names_the_database_path` — error text is the actionable message asked for and includes the db path. - [x] `lock_file_is_created_beside_the_database_path` — sidecar `.lock` file appears next to the db path. - [x] `cargo fmt --all -- --check`, `cargo check-all` (clippy `--all-targets --workspace -D warnings`), `cargo test-all` all green locally (pre-commit hook ran the same three gates and passed). `nunciod` unit suite: 131 passed / 0 failed / 0 ignored; full workspace `cargo test-all` (unit + integration + E2E, all crates): also green, 0 ignored. ## Notes / follow-ups - Encountered one pre-existing flaky test unrelated to this change: `nuncio-cli`'s `mail_and_folder_report_honest_errors_when_daemon_unreachable` (binds an ephemeral port, drops it, then expects the connect to that now-free port to fail) failed once on a background port-reuse race and passed on every other run, including inside the pre-commit hook that gated this commit. Not something this PR should fix; flagging in case it's worth a tracking issue for hardening that test's port-reuse assumption.
Sign in to join this conversation.