Skip to content

Handle stale storage locks after crashes#7

Merged
VectoDE merged 1 commit intomainfrom
codex/fix-stale-lock-file-handling
Oct 21, 2025
Merged

Handle stale storage locks after crashes#7
VectoDE merged 1 commit intomainfrom
codex/fix-stale-lock-file-handling

Conversation

@VectoDE
Copy link
Copy Markdown
Owner

@VectoDE VectoDE commented Oct 21, 2025

Summary

  • ensure file lock acquisition times out and cleans up stale lock files
  • record lock metadata when acquiring the storage lock so stale locks can be detected

Testing

  • go test ./...

https://chatgpt.com/codex/tasks/task_e_68f7e94c32dc832cb9683156aa138095

@VectoDE VectoDE changed the base branch from codex/fix-concurrent-saves-in-password-store to main October 21, 2025 20:17
@VectoDE VectoDE merged commit 32fd735 into main Oct 21, 2025
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/storage/store.go
Comment on lines 199 to +205
if errors.Is(err, os.ErrExist) {
time.Sleep(50 * time.Millisecond)
info, statErr := os.Stat(s.lockPath)
if statErr == nil {
if time.Since(info.ModTime()) > lockStaleAgeThreshold {
if removeErr := os.Remove(s.lockPath); removeErr == nil {
continue
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Removing stale lock purely by age can free an active writer

The new stale-lock handling deletes the lock file whenever its ModTime exceeds lockStaleAgeThreshold, without checking the PID written into the file. If a legitimate save operation holds the lock for more than 30 seconds (e.g. slow disk, a large file, or the process is paused), a concurrent call will consider the lock stale, delete the file, and enter the critical section while the first writer is still updating the store. This reintroduces the concurrent write corruption that the locking mechanism is meant to prevent. The stale check should verify that the recorded PID is no longer running (or otherwise ensure the lock is truly abandoned) before removing it, or avoid expiring active locks by age alone.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant