Skip to content

Recover from a lock file that cannot be decoded - #416

Open
PerryLink wants to merge 1 commit into
caddyserver:masterfrom
PerryLink:fix/unreadable-lockfile-recovery
Open

PerryLink wants to merge 1 commit into
caddyserver:masterfrom
PerryLink:fix/unreadable-lockfile-recovery

Conversation

@PerryLink

Copy link
Copy Markdown

What

FileStorage.obtainLock() refuses to work when the lock file exists but cannot be JSON-decoded —
and that refusal happens before the stale-lock check:

// filestorage.go:210-232
if err := json.Unmarshal(b, &meta); err != nil {
    return nil, fmt.Errorf("decoding lockfile contents: %w", err)   // returns here
}
...
// filestorage.go:244-256 -- stale lock removal, never reached

A lock file left truncated or zero-filled by a crash during refresh therefore becomes permanent: every
node fails to acquire the lock for that key forever, and only a manual delete recovers it.

Measured on local NTFS with a hand-written corrupt file and the public API:

lock contents b'{"created":'                  -> decoding lockfile contents: unexpected EOF
lock contents 98 zero bytes                   -> invalid character '\x00' looking for beginning of value
lock contents {"updated":"not-a-timestamp"}   -> parsing time "not-a-timestamp" ...

Three consecutive Lock() attempts fail with identical errors and the file keeps the same size and
mtime, so no amount of retrying helps.

The change

Treat an undecodable lock file the same way the code already treats an empty one (the policy merged
in #232): retry a few times, then consider it stale and remove it. io.EOF keeps the existing
"Empty lockfile" log; other decode errors get "Unreadable lockfile"; the partially decoded metadata is
zeroed so a corrupt file cannot look fresh.

This follows the file's own design note (filestorage.go:56-64): imperfect mutual exclusion is
preferable to a permanent loop.

Verification

New table test with three corrupt variants, run in parallel:

  • before: all three fail with the errors quoted above;
  • after: all three pass in 1.8s; -count=3 passes too;
  • go test -count=1 ./... passes; gofmt/go vet clean; git apply --check on a pristine master.

Open PR #337 changes how lock files are written (write+rename) and is orthogonal — this patch is about
recovering from a file that is already corrupt on disk.

Commits are signed off, as CONTRIBUTING.md asks.

Signed-off-by: PerryLink <255665900+PerryLink@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant