f - Prevent stale fs-store writes
What the developer wrote
Authored by Elias Rohrer
f - Prevent stale fs-store writes
Move the version-allocation ordering note to the allocation it
describes so the cleanup invariant is easier to follow.
Co-Authored-By: HAL 9000
What changed, and why it matters
This is a small follow-up patch (marked with 'f -') that reorders code and adds a comment in a filesystem persistence module. The change moves version allocation to happen while a lock-map mutex is still held, preventing a theoretical race where a cleanup routine could remove a lock entry after a version number is reserved but before the lock reference is copied. The commit message frames this as preventing 'stale fs-store writes.' The actual code behavior change is minimal and the patch is conservative.
Treat as a minor hardening fix. Review the clean_locks implementation to confirm the race window exists and is fully closed by this ordering change. Consider adding a regression test that exercises concurrent version allocation and lock cleanup. No urgent action is warranted based solely on this diff.
Security signals we found
Race condition in lock-map cleanup (stated by commit message/comment, not independently verified)
Potential stale write to filesystem persistence store (stated by commit message)
Version counter ordering relative to mutex guard
Follow-up fix ('f -' prefix in commit title)
Evidence from the diff
In lightning-persister/src/fs_store/common.rs, get_new_version_and_lock_ref previously fetched a new version, then acquired the locks map mutex, then cloned the per-file RwLock. The patch moves the version allocation (self.next_version.fetch_add) to occur while outer_lock (the MutexGuard over the locks map) is held. The stated invariant is that clean_locks cannot remove a locks-map entry between version reservation and lock-reference cloning. The diff is +3/-2 and primarily comment relocation; the functional change is the ordering of fetch_add relative to the mutex acquisition. No test changes or advisory references are supplied.
Changed components
lightning-persister/src/fs_store/common.rsFilesystemStoreState::get_new_version_and_lock_refFilesystemStore lock-map cleanup (clean_locks)Inspect captured patch +3 / −2
diff --git a/lightning-persister/src/fs_store/common.rs b/lightning-persister/src/fs_store/common.rs
index 5c73ad1..16a1352 100644
--- a/lightning-persister/src/fs_store/common.rs
+++ b/lightning-persister/src/fs_store/common.rs
@@ -93,14 +93,15 @@ impl FilesystemStoreState {
fn get_new_version_and_lock_ref(&self, dest_file_path: PathBuf) -> (Arc<RwLock<u64>>, u64) {
let mut outer_lock = self.inner.locks.lock().unwrap();
+ // Allocate the version while holding the lock map mutex so that clean_locks cannot remove the entry after a
+ // version has been reserved but before its lock reference is cloned.
let version = self.next_version.fetch_add(1, Ordering::Relaxed);
if version == u64::MAX {
panic!("FilesystemStore version counter overflowed");
}
// Get a reference to the inner lock. We do this early so that the arc can double as an in-flight counter for
- // cleaning up unused locks. Allocate the version while holding the lock map mutex so that clean_locks cannot
- // remove the entry after a version has been reserved but before its lock reference is cloned.
+ // cleaning up unused locks.
let inner_lock_ref = Arc::clone(&outer_lock.entry(dest_file_path).or_default());
(inner_lock_ref, version)
Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.