Make writes atomic on *nix platforms.

And remove an unnecessary fsync on the backup file after rename.

Author: valentinewallace

lightning-data-persister/Cargo.toml

@@ -10,6 +10,7 @@ Utilities to manage channel data persistence and retrieval.
 [dependencies]
 bitcoin = "0.23"
 lightning = { version = "0.0.11", path = "../lightning" }
+libc = "0.2"
 
 [dev-dependencies.bitcoin]
 version = "0.23"

lightning-data-persister/src/lib.rs

@@ -1,5 +1,6 @@
 extern crate lightning;
 extern crate bitcoin;
+extern crate libc;
 
 use lightning::ln::data_persister::ChannelDataPersister;
 use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr};
@@ -14,6 +15,9 @@ use std::io::{Error, ErrorKind, Cursor};
 use std::collections::HashMap;
 use std::marker::PhantomData;
 
+#[cfg(not(target_os = "windows"))]
+use std::os::unix::io::AsRawFd;
+
 /// FilesystemPersister can persist channel data on disk on Linux machines, where
 /// each channel's data is stored in a file named after its funding outpoint.
 ///
@@ -42,8 +46,8 @@ impl<ChanSigner: ChannelKeys + Readable + Writeable> FilesystemPersister<ChanSig
 	}
 
 	fn get_full_filepath(&self, funding_txo: OutPoint) -> String {
-		let path = Path::new(&self.path_to_channel_data);
-		let mut path_buf = path.to_path_buf();
+		let dir_path = Path::new(&self.path_to_channel_data);
+		let mut path_buf = dir_path.to_path_buf();
 		path_buf.push(format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index));
 		path_buf.to_str().unwrap().to_string()
 	}
@@ -55,10 +59,9 @@ impl<ChanSigner: ChannelKeys + Readable + Writeable> FilesystemPersister<ChanSig
 	fn write_channel_data(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChanSigner>) -> std::io::Result<()> {
 		// Do a crazy dance with lots of fsync()s to be overly cautious here...
 		// We never want to end up in a state where we've lost the old data, or end up using the
-		// old data on power loss after we've returned
-		// Note that this actually *isn't* enough (at least on Linux)! We need to fsync an fd with
-		// the containing dir, but Rust doesn't let us do that directly, sadly. TODO: Fix this with
-		// the libc crate!
+		// old data on power loss after we've returned.
+		// The way to atomically write a file on Unix platforms is:
+		// open(tmpname), write(tmpfile), fsync(tmpfile), close(tmpfile), fsync(dir), rename(), fsync(dir)
 		let filename = self.get_full_filepath(funding_txo);
 		let tmp_filename = filename.clone() + ".tmp";
 
@@ -88,13 +91,17 @@ impl<ChanSigner: ChannelKeys + Readable + Writeable> FilesystemPersister<ChanSig
 			}
 		}
 		fs::rename(&tmp_filename, &filename)?;
-		{
-			let f = fs::OpenOptions::new().write(true).open(&filename)?;
-			f.sync_all()?;
-		}
 		if need_bk {
 			fs::remove_file(&bk_filename)?;
 		}
+		// Fsync the parent directory on Unix.
+		#[cfg(not(target_os = "windows"))]
+		{
+			let path_str = filename.clone();
+			let path = Path::new(&path_str).parent().unwrap();
+			let dir_file = fs::OpenOptions::new().read(true).open(path)?;
+			unsafe { libc::fsync(dir_file.as_raw_fd()); }
+		}
 		Ok(())
 	}
 }