Skip to content

fix(lock): check ownership before releasing lock in cleanup#65

Open
kitsuyui wants to merge 1 commit into
mainfrom
fix/audit-lock-cleanup-no-ownership-check-001
Open

fix(lock): check ownership before releasing lock in cleanup#65
kitsuyui wants to merge 1 commit into
mainfrom
fix/audit-lock-cleanup-no-ownership-check-001

Conversation

@kitsuyui

Copy link
Copy Markdown
Member

Summary

cleanup_gibo_update_lock() deleted the lock directory unconditionally, based only on whether gibo_update_lock_dir was set. This created a race condition:

  1. Process A acquires the lock and starts gibo update
  2. gibo update runs longer than lock_stale_seconds (600 s)
  3. Process B detects a stale lock, removes it, and acquires its own lock
  4. Process A finishes and its EXIT trap calls cleanup_gibo_update_lock()
  5. Process A deletes the lock directory that Process B now owns
  6. A third process can now acquire the lock simultaneously with Process B

The pid_ts file already records the owner's PID at lock-acquisition time. This PR reads pid_ts in the cleanup function and only removes the lock directory when the recorded PID matches the current process ($$).

Behavior change

Scenario Before After
Normal cleanup (no stale eviction) Delete lock Delete lock (PID matches $$)
Stale eviction occurred; another process now owns lock Delete lock (incorrect) Leave lock in place (correct)
pid_ts unreadable or empty Delete lock Leave lock in place (safe; stale detection recovers within 600 s)

Verification

  • No local test runner; CI test runs via GitHub Actions (Example workflow)
  • check-pr-collateral.py — clean

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