statefile: use stable inode for write lock - #1149
Conversation
The previous implementation used atomic_write_with_perms to create the lockfile, which replaces the file via rename on every call. This meant that concurrent callers could flock different inodes, defeating the purpose of the lock. Let's replace that with OpenOptions create-or-open, which only creates the file if it doesn't already exist and opens the existing one otherwise, ensuring all callers lock the same inode. The lack of atomic writing here should not be an issue as the file has no content anyway.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (10)
🧰 Additional context used📓 Path-based instructions (1)Backend and state-file changes affect persistent bootloader update state. Preserve compatibility with existing state and ensure updates remain atomic and recoverable after interruption.⚙️ CodeRabbit configuration file Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThe statefile backend now creates and opens the write-lock file in one operation with write/create options and ChangesLock File Acquisition
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change makes lockfile creation reuse a stable inode so concurrent callers coordinate on the same lock; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
Full details: Commit Message ConventionExplanation The PR adds one non-merge commit. Its subject is ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Ouch. |
3 participants
The previous implementation used atomic_write_with_perms to create the lockfile, which replaces the file via rename on every call. This meant that concurrent callers could flock different inodes, defeating the purpose of the lock.
Let's replace that with OpenOptions create-or-open, which only creates the file if it doesn't already exist and opens the existing one otherwise, ensuring all callers lock the same inode. The lack of atomic writing here should not be an issue as the file has no content anyway.
Originally pointed out by the bot on #1138, see #1138 (comment)