[PastaPastaPasta]

Follow-up to #1257. On Unix, LockFile's Drop deletes the lock file while it still holds the lock. An opener that locks a deleted file then notices, because its file's device and inode no longer match the file at the path, and it opens again. On Windows that check is skipped: std exposes no file identity there on stable Rust. Depending on the Windows version and file system, deleting a file that is still open either frees the name at once (POSIX semantics) or leaves the file delete-pending until its last handle closes. With POSIX semantics, an opener that opened the old file can lock it after the holder drops it, while a later opener creates and locks a new file at the same path, and both hold the lock. With delete-pending, a later opener fails to open the path until the earlier handles close, instead of being refused by the lock.

Off Unix, the lock file is now kept on drop instead. As long as nothing else deletes or replaces it, every opener opens the same file, so the OS lock alone excludes them. The next holder truncates the file once it has the lock and writes its PID, as before.

Tests:

• test_lock_file_removed_on_drop runs on every platform: it asserts that the file is removed after drop on Unix and that it stays elsewhere.
• test_lock_lifecycle in storage/mod.rs asserts removal on Unix and presence elsewhere.
• Locally on Linux, the dash-spv lib tests and the dashd restart tests pass, and clippy with -D warnings passes for the workspace and for x86_64-pc-windows-gnu with --all-targets. The Windows side runs in CI's Windows job.

🤖 Generated with Claude Code

PR Hygiene · 77d1f7a

• Bots — coderabbitai not yet — /skip-bots proceeds without the ones not yet reported
• Self-review — address xdustinface requested changes, then post /self-reviewed
• Build running
• Approvals
  • files with no dedicated owner (CHANGELOG.md) — QuantumExplorer or ZocoLini or xdustinface
  • dash-spv (dash-spv/src/storage/lockfile.rs, dash-spv/src/storage/mod.rs) — QuantumExplorer or ZocoLini or xdustinface

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 19 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: dashpay/rust-dashcore
• Branch: pasta/spv-lockfile-windows
• File: .coderabbit.yaml
• Commit: 77d1f7a
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 38e63153-9404-4cfd-8d43-083a831155d4

Commits

Reviewing files that changed from the base of the PR and between 0e60269 and 77d1f7a.

Files selected for processing: 3

• CHANGELOG.md
• dash-spv/src/storage/lockfile.rs
• dash-spv/src/storage/mod.rs

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: dashpay/rust-dashcore
• Branch: pasta/spv-lockfile-windows
• File: .coderabbit.yaml
• Commit: 0e60269
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 211c7905-e725-4439-947b-d2eedf0ef891

Commits

Reviewing files that changed from the base of the PR and between bab8b10 and 0e60269.

Files selected for processing: 3

• CHANGELOG.md
• dash-spv/src/storage/lockfile.rs
• dash-spv/src/storage/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

On Unix, dropping a lock removes its file. On non-Unix platforms, the file remains, and a later holder records its PID. The lock-file lifecycle tests now check this platform-specific behavior.

Changes: Platform-specific lock-file lifecycle

Layer / File(s)	Summary
Lock-file lifecycle by platform dash-spv/src/storage/lockfile.rs, dash-spv/src/storage/mod.rs, CHANGELOG.md	Lock drops remove the file on Unix and retain it on non-Unix platforms. Tests check file retention and PID recording after reacquisition on non-Unix platforms, and removal on Unix. The changelog records the non-Unix behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

• dashpay/rust-dashcore#1257: Changes the same lock lifecycle, including truncation after lock acquisition and behavior when a locked file path is unlinked.
• dashpay/rust-dashcore#1178: Addresses the same lifecycle from the client shutdown side by waiting for detached tasks that retain the storage lock.

Suggested reviewers: zocolini

Merge Risk: Minimal · previous review 0e602

No actionable merge-blocking issue remains; normal Windows CI validation should complete before release.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: retaining the dash-spv data-directory lock file after drop on non-Unix platforms.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[github-actions]

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.71%. Comparing base (6d291cd) to head (77d1f7a).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1263      +/-   ##
==========================================
+ Coverage   80.70%   80.71%   +0.01%     
==========================================
  Files         291      291              
  Lines       77763    77764       +1     
==========================================
+ Hits        62755    62767      +12     
+ Misses      15008    14997      -11     

Flag	Coverage Δ
core	78.30% <ø> (ø)
ffi	58.20% <ø> (ø)
rpc	52.05% <ø> (ø)
spv	93.75% <100.00%> (+0.05%)	⬆️
wallet	82.88% <ø> (ø)

Files with missing lines	Coverage Δ
dash-spv/src/storage/lockfile.rs	92.21% <100.00%> (+0.04%)	⬆️
dash-spv/src/storage/mod.rs	90.90% <ø> (ø)

... and 4 files with indirect coverage changes

[PastaPastaPasta]

/self-reviewed

[github-actions]

Ready for review — files with no dedicated owner: QuantumExplorer or ZocoLini or xdustinface · dash-spv: QuantumExplorer or ZocoLini or xdustinface.
Full checklist in the description.

[xdustinface]

1. Gate path and Drop on Unix instead of suppressing dead_code. Off Unix, LockFile::path is only read in Drop, whose body is now empty there, so #[cfg_attr(not(unix), allow(dead_code))] just hides a dead field. Put #[cfg(unix)] on the path field, on the path, initialiser in LockFile::new, and on the whole impl Drop for LockFile, and drop the inner #[cfg(unix)] on remove_file. I checked this locally: clippy with -D warnings and the storage tests pass on Unix, and a cfg-flipped build of lockfile.rs is clean on the non-Unix side too. Once Drop only exists on Unix, the Windows explanation in its doc comment should move to the LockFile struct doc, and the non-Unix is_at doc should stop pointing at LockFile's Drop.

2. Drop test_lock_file_kept_on_drop, it adds no coverage. Its "file stays" assert is already in test_lock_lifecycle (the #[cfg(not(unix))] assert you added in storage/mod.rs), and re-acquiring after drop is covered on every platform by test_lock_released_on_drop. The read_pid assert can't show a takeover either: both holders are the same process, so the PID matches whether or not the file was truncated (a_stale_lock_file_holds_only_the_new_pid covers truncation). Instead, turn test_lock_file_removed_on_drop into one test for all platforms with #[cfg(unix)] assert!(!lock_path.exists()) and #[cfg(not(unix))] assert!(lock_path.exists()), the same split you used in test_lock_lifecycle.

[github-actions]

Bots are done — your move: address xdustinface requested changes, then post /self-reviewed.
Full checklist in the description.

[PastaPastaPasta]

Thanks, both done in 4b3f047. The path field, its initialiser and the whole impl Drop for LockFile are now #[cfg(unix)], so the allow(dead_code) and the inner cfg on remove_file are gone. The Windows explanation has moved to the LockFile doc, and the non-Unix is_at doc points there instead of at Drop. test_lock_file_kept_on_drop is removed, and test_lock_file_removed_on_drop now runs on every platform, asserting the file is gone after drop on Unix and still there elsewhere. Clippy with -D warnings passes for the workspace and for x86_64-pc-windows-gnu (--all-targets). The dash-spv lib tests and the dashd restart tests pass on Linux.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[github-actions]

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[github-actions]

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[github-actions]

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.