Skip to content

Commit 07b229c

Browse files
Benehikoclaude
andcommitted
refactor(posixage/flock): route heartbeat logs through caller's logger
Replace log/slog with the project's logging.Logger interface so the library no longer writes to a default sink. The heartbeat goroutine resolves its logger via logging.FromContext, falling back to a noop when the caller hasn't set one. posixage's fileStore plumbs its configured logger through with logging.WithLogger so the existing WithLogger option on store.New propagates transparently without any new parameters on TryLock/TryRLock. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8dc2881 commit 07b229c

2 files changed

Lines changed: 52 additions & 13 deletions

File tree

store/posixage/internal/flock/flock.go

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,35 @@ import (
1818
"context"
1919
"errors"
2020
"io/fs"
21-
"log/slog"
2221
"os"
2322
"sync"
2423
"time"
2524

2625
"github.com/cenkalti/backoff/v5"
26+
"github.com/docker/secrets-engine/x/logging"
2727
)
2828

29+
// noopLogger discards every record. It is the [logging.Logger]
30+
// implementation used when no logger is provided in the context passed
31+
// to [TryLock] or [TryRLock], so the package never writes to the
32+
// caller's stderr unsolicited.
33+
type noopLogger struct{}
34+
35+
func (noopLogger) Printf(string, ...any) {}
36+
func (noopLogger) Warnf(string, ...any) {}
37+
func (noopLogger) Errorf(string, ...any) {}
38+
39+
// loggerFromCtx returns the [logging.Logger] stored on ctx by
40+
// [logging.WithLogger], or a [noopLogger] when none is set. The library
41+
// must never log to a default sink — silence is the safe default for a
42+
// dependency.
43+
func loggerFromCtx(ctx context.Context) logging.Logger {
44+
if l, err := logging.FromContext(ctx); err == nil {
45+
return l
46+
}
47+
return noopLogger{}
48+
}
49+
2950
var (
3051
ErrLockUnsuccessful = errors.New("store is locked")
3152
ErrUnlockUnsuccessful = errors.New("could not unlock store")
@@ -144,9 +165,10 @@ func isCurrentLockFile(fl *os.File, root *os.Root) (bool, error) {
144165
}
145166

146167
func tryLock(ctx context.Context, root *os.Root, exclusive bool) (UnlockFunc, error) {
168+
logger := loggerFromCtx(ctx)
147169
fl, err := acquireOnce(root, exclusive)
148170
if err == nil {
149-
return startHeartbeat(fl, root), nil
171+
return startHeartbeat(fl, root, logger), nil
150172
}
151173
firstErr := errors.Join(ErrLockUnsuccessful, err)
152174

@@ -162,19 +184,23 @@ func tryLock(ctx context.Context, root *os.Root, exclusive bool) (UnlockFunc, er
162184
if err != nil {
163185
return nil, err
164186
}
165-
return startHeartbeat(fl, root), nil
187+
return startHeartbeat(fl, root, logger), nil
166188
}
167189

168190
// startHeartbeat launches the modtime-refresh goroutine for a locked file
169191
// and returns an [UnlockFunc] that stops the goroutine, waits for it to
170192
// exit, and then unlocks/closes the file. The wait ensures the goroutine
171193
// never touches the fd after [unlockFile] closes it.
172-
func startHeartbeat(fl *os.File, root *os.Root) UnlockFunc {
194+
//
195+
// The supplied logger is used by the goroutine to surface truncate
196+
// failures and inode-mismatch hijacks. A [noopLogger] is acceptable when
197+
// the caller has no logging plumbed.
198+
func startHeartbeat(fl *os.File, root *os.Root, logger logging.Logger) UnlockFunc {
173199
hbCtx, stop := context.WithCancel(context.Background())
174200
done := make(chan struct{})
175201
go func() {
176202
defer close(done)
177-
heartbeat(hbCtx, fl, root)
203+
heartbeat(hbCtx, fl, root, logger)
178204
}()
179205
return sync.OnceValue(func() error {
180206
stop()
@@ -194,12 +220,13 @@ func startHeartbeat(fl *os.File, root *os.Root) UnlockFunc {
194220
// file at the lock-file path. A mismatch means we have been hijacked
195221
// (heartbeat starved past [staleThreshold] long enough for recovery to
196222
// fire). There is no in-band way to fail the caller's outstanding
197-
// operation, so the mismatch is logged via [slog] and the goroutine
198-
// keeps running — surfacing the hijack is the best we can do.
223+
// operation, so the mismatch is logged via the supplied [logging.Logger]
224+
// and the goroutine keeps running — surfacing the hijack is the best we
225+
// can do.
199226
//
200227
// The goroutine returns when ctx is canceled by [startHeartbeat]'s
201228
// returned [UnlockFunc].
202-
func heartbeat(ctx context.Context, fl *os.File, root *os.Root) {
229+
func heartbeat(ctx context.Context, fl *os.File, root *os.Root, logger logging.Logger) {
203230
ticker := time.NewTicker(heartbeatInterval)
204231
defer ticker.Stop()
205232
for {
@@ -208,16 +235,16 @@ func heartbeat(ctx context.Context, fl *os.File, root *os.Root) {
208235
return
209236
case <-ticker.C:
210237
if err := fl.Truncate(0); err != nil {
211-
slog.Warn("flock heartbeat: truncate failed", "err", err)
238+
logger.Warnf("flock heartbeat: truncate failed: %v", err)
212239
continue
213240
}
214241
same, err := isCurrentLockFile(fl, root)
215242
if err != nil {
216-
slog.Warn("flock heartbeat: inode verify failed", "err", err)
243+
logger.Warnf("flock heartbeat: inode verify failed: %v", err)
217244
continue
218245
}
219246
if !same {
220-
slog.Warn("flock heartbeat: lock file inode changed under us; lock has likely been hijacked")
247+
logger.Warnf("flock heartbeat: lock file inode changed under us; lock has likely been hijacked")
221248
}
222249
}
223250
}
@@ -254,10 +281,16 @@ func retryLock(ctx context.Context, root *os.Root, exclusive bool) (*os.File, er
254281
// recovery is skipped when ctx has been canceled. If recovery fails,
255282
// manual intervention may be required.
256283
//
284+
// The heartbeat goroutine surfaces truncate failures and hijack
285+
// detections through the [logging.Logger] stored on ctx via
286+
// [logging.WithLogger]. When no logger is set, those events are dropped
287+
// silently — the package never writes to a default sink.
288+
//
257289
// On success, the returned [UnlockFunc] MUST be called exactly once to
258290
// release the lock, close the file descriptor, and stop the heartbeat
259291
// goroutine. The idiomatic pattern is to defer it immediately:
260292
//
293+
// ctx = logging.WithLogger(ctx, myLogger) // optional
261294
// unlock, err := flock.TryLock(ctx, root)
262295
// if err != nil {
263296
// return err
@@ -284,10 +317,16 @@ func TryLock(ctx context.Context, root *os.Root) (UnlockFunc, error) {
284317
// recovery is skipped when ctx has been canceled. If recovery fails,
285318
// manual intervention may be required.
286319
//
320+
// The heartbeat goroutine surfaces truncate failures and hijack
321+
// detections through the [logging.Logger] stored on ctx via
322+
// [logging.WithLogger]. When no logger is set, those events are dropped
323+
// silently — the package never writes to a default sink.
324+
//
287325
// On success, the returned [UnlockFunc] MUST be called exactly once to
288326
// release the lock, close the file descriptor, and stop the heartbeat
289327
// goroutine. The idiomatic pattern is to defer it immediately:
290328
//
329+
// ctx = logging.WithLogger(ctx, myLogger) // optional
291330
// unlock, err := flock.TryRLock(ctx, root)
292331
// if err != nil {
293332
// return err

store/posixage/store.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ var _ store.Store = &fileStore[store.Secret]{}
6363
func (f *fileStore[T]) tryLock(ctx context.Context) (func(), error) {
6464
f.l.Lock()
6565

66-
unlock, err := flock.TryLock(ctx, f.filesystem)
66+
unlock, err := flock.TryLock(logging.WithLogger(ctx, f.logger), f.filesystem)
6767
if err != nil {
6868
f.l.Unlock()
6969
return nil, err
@@ -89,7 +89,7 @@ func (f *fileStore[T]) tryLock(ctx context.Context) (func(), error) {
8989
func (f *fileStore[T]) tryRLock(ctx context.Context) (func(), error) {
9090
f.l.RLock()
9191

92-
unlock, err := flock.TryRLock(ctx, f.filesystem)
92+
unlock, err := flock.TryRLock(logging.WithLogger(ctx, f.logger), f.filesystem)
9393
if err != nil {
9494
f.l.RUnlock()
9595
return nil, err

0 commit comments

Comments
 (0)