Replace race-y metadata call with DirEntry::metadata

Previously, the internal `dir_entry_is_key` method would take a path
argument, which would risk a race when the respective entry was modified
in the time between the original `fs::read_dir` call and the
`Path::metadata` call.

Here, we instead have it take a `DirEntry` argument, which--at least on
some platforms--should allow us to avoid this race as
`DirEntry::metadata` would return the original (cached) metadata.

Author: tnull

lightning-persister/src/fs_store.rs

@@ -316,11 +316,11 @@ impl KVStoreSync for FilesystemStore {
 			let entry = entry?;
 			let p = entry.path();
 
-			if !dir_entry_is_key(&p)? {
+			if !dir_entry_is_key(&entry)? {
 				continue;
 			}
 
-			let key = get_key_from_dir_entry(&p, &prefixed_dest)?;
+			let key = get_key_from_dir_entry_path(&p, &prefixed_dest)?;
 
 			keys.push(key);
 		}
@@ -331,7 +331,8 @@ impl KVStoreSync for FilesystemStore {
 	}
 }
 
-fn dir_entry_is_key(p: &Path) -> Result<bool, lightning::io::Error> {
+fn dir_entry_is_key(dir_entry: &fs::DirEntry) -> Result<bool, lightning::io::Error> {
+	let p = dir_entry.path();
 	if let Some(ext) = p.extension() {
 		#[cfg(target_os = "windows")]
 		{
@@ -346,7 +347,7 @@ fn dir_entry_is_key(p: &Path) -> Result<bool, lightning::io::Error> {
 		}
 	}
 
-	let metadata = p.metadata().map_err(|e| {
+	let metadata = dir_entry.metadata().map_err(|e| {
 		let msg = format!(
 			"Failed to list keys at path {}: {}",
 			PrintableString(p.to_str().unwrap_or_default()),
@@ -377,7 +378,7 @@ fn dir_entry_is_key(p: &Path) -> Result<bool, lightning::io::Error> {
 	Ok(true)
 }
 
-fn get_key_from_dir_entry(p: &Path, base_path: &Path) -> Result<String, lightning::io::Error> {
+fn get_key_from_dir_entry_path(p: &Path, base_path: &Path) -> Result<String, lightning::io::Error> {
 	match p.strip_prefix(&base_path) {
 		Ok(stripped_path) => {
 			if let Some(relative_path) = stripped_path.to_str() {
@@ -435,24 +436,27 @@ impl MigratableKVStore for FilesystemStore {
 		let mut keys = Vec::new();
 
 		'primary_loop: for primary_entry in fs::read_dir(prefixed_dest)? {
-			let primary_path = primary_entry?.path();
+			let primary_entry = primary_entry?;
+			let primary_path = primary_entry.path();
 
-			if dir_entry_is_key(&primary_path)? {
+			if dir_entry_is_key(&primary_entry)? {
 				let primary_namespace = String::new();
 				let secondary_namespace = String::new();
-				let key = get_key_from_dir_entry(&primary_path, prefixed_dest)?;
+				let key = get_key_from_dir_entry_path(&primary_path, prefixed_dest)?;
 				keys.push((primary_namespace, secondary_namespace, key));
 				continue 'primary_loop;
 			}
 
 			// The primary_entry is actually also a directory.
 			'secondary_loop: for secondary_entry in fs::read_dir(&primary_path)? {
-				let secondary_path = secondary_entry?.path();
+				let secondary_entry = secondary_entry?;
+				let secondary_path = secondary_entry.path();
 
-				if dir_entry_is_key(&secondary_path)? {
-					let primary_namespace = get_key_from_dir_entry(&primary_path, prefixed_dest)?;
+				if dir_entry_is_key(&secondary_entry)? {
+					let primary_namespace =
+						get_key_from_dir_entry_path(&primary_path, prefixed_dest)?;
 					let secondary_namespace = String::new();
-					let key = get_key_from_dir_entry(&secondary_path, &primary_path)?;
+					let key = get_key_from_dir_entry_path(&secondary_path, &primary_path)?;
 					keys.push((primary_namespace, secondary_namespace, key));
 					continue 'secondary_loop;
 				}
@@ -462,12 +466,12 @@ impl MigratableKVStore for FilesystemStore {
 					let tertiary_entry = tertiary_entry?;
 					let tertiary_path = tertiary_entry.path();
 
-					if dir_entry_is_key(&tertiary_path)? {
+					if dir_entry_is_key(&tertiary_entry)? {
 						let primary_namespace =
-							get_key_from_dir_entry(&primary_path, prefixed_dest)?;
+							get_key_from_dir_entry_path(&primary_path, prefixed_dest)?;
 						let secondary_namespace =
-							get_key_from_dir_entry(&secondary_path, &primary_path)?;
-						let key = get_key_from_dir_entry(&tertiary_path, &secondary_path)?;
+							get_key_from_dir_entry_path(&secondary_path, &primary_path)?;
+						let key = get_key_from_dir_entry_path(&tertiary_path, &secondary_path)?;
 						keys.push((primary_namespace, secondary_namespace, key));
 					} else {
 						debug_assert!(