From 3c981c192fd23879a2e8531391e0971b3d1ecfd0 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Tue, 19 May 2026 21:32:51 -0400 Subject: [PATCH] chore(db): log lock metadata write failures and explain lock file lifetime Address review feedback on the data directory lock. The metadata written into the lock file is informational only, but silently swallowing a write failure leaves a future contender with no clues to report. Log it at debug level. Also leave a comment next to the lock acquisition explaining why we intentionally do not delete the lock file on release: flock is keyed by inode, and any close-then-unlink ordering opens a race where two processes can each hold a flock on a different inode that lives at the same path. Co-Authored-By: Charm Crush --- internal/db/datadirlock.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/db/datadirlock.go b/internal/db/datadirlock.go index 899e82958267a668504ec11e96fa4e7c0bf8972e..914933503fd795dd13a2052af76a8cd597015c04 100644 --- a/internal/db/datadirlock.go +++ b/internal/db/datadirlock.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "os" "path/filepath" "strconv" @@ -63,13 +64,17 @@ func acquireDataDirLock(dataDir string) (*dataDirLock, error) { // Record ownership metadata so a contending process can identify // us. Failures here are non-fatal: the OS-level lock is what // actually guarantees mutual exclusion, and a missing/partial JSON - // payload only degrades diagnostics. + // payload only degrades the diagnostic a contender prints. if err := writeOwnerInfo(path); err != nil { - // Best-effort; log via stderr only when running in a debug - // context would be invasive here, so we silently swallow. - _ = err + slog.Debug("Failed to write data-dir owner info", "path", path, "error", err) } + // The lock file itself is intentionally never unlinked. flock is + // keyed by inode, not by path, and any close-then-unlink (or + // unlink-then-close) ordering opens a window where two processes + // can each hold a flock on a different inode that lives at the + // same path. Leaving the file in place lets every acquirer see + // the same inode and lets the kernel arbitrate correctly. return &dataDirLock{release: release}, nil }