From 8227ce941a440da958146698f0f695ef2c69379e Mon Sep 17 00:00:00 2001
From: Severus Sneep <38492429+ssneep@users.noreply.github.com>
Date: Tue, 24 Apr 2018 22:03:01 +0200
Subject: [PATCH] Use a lock-directory- instead of a lock-file- and backup prev
 wallet.dat. (#982)

It looks like lockfile implementation did still suffer from a race
condition. Only when creating the file with O_EXCL, file creation fails
if the file does already exist. One could use O_EXCL, but Windows
might use another flag to achieve the same. Moreover, O_EXCL doesn't
work as expected if the files are accessed over NFS.
In contrast, mkdir() is atomic in every of the mentioned cases.
Aside from using a lockdirectory, this patch also backups the current
wallet.dat contents to wallet.bck in case writing a possibly hefty
amount of JSON to the new wallet.dat fails. It is hoped that at least
one of the .bck and .dat files has usable contents in case failure.
---
 wallet/src/types.rs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/wallet/src/types.rs b/wallet/src/types.rs
index 572bcb43..30d2e069 100644
--- a/wallet/src/types.rs
+++ b/wallet/src/types.rs
@@ -18,7 +18,7 @@ use std::fmt;
 use std::fmt::Display;
 use uuid::Uuid;
 use std::convert::From;
-use std::fs::{self, File, OpenOptions};
+use std::fs::{self, File};
 use std::io::{Read, Write};
 use std::path::Path;
 use std::path::MAIN_SEPARATOR;
@@ -47,6 +47,7 @@ use util::secp::key::PublicKey;
 use util::LOGGER;
 
 const DAT_FILE: &'static str = "wallet.dat";
+const BCK_FILE: &'static str = "wallet.bck";
 const LOCK_FILE: &'static str = "wallet.lock";
 const SEED_FILE: &'static str = "wallet.seed";
 
@@ -521,21 +522,19 @@ impl WalletData {
 		});
 
 		let data_file_path = &format!("{}{}{}", data_file_dir, MAIN_SEPARATOR, DAT_FILE);
+		let backup_file_path = &format!("{}{}{}", data_file_dir, MAIN_SEPARATOR, BCK_FILE);
 		let lock_file_path = &format!("{}{}{}", data_file_dir, MAIN_SEPARATOR, LOCK_FILE);
 
 		info!(LOGGER, "Acquiring wallet lock ...");
 
 		let action = || {
 			trace!(LOGGER, "making lock file for wallet lock");
-			OpenOptions::new()
-				.write(true)
-				.create_new(true)
-				.open(lock_file_path)
+			fs::create_dir(lock_file_path)
 		};
 
 		// use tokio_retry to cleanly define some retry logic
 		let mut core = reactor::Core::new().unwrap();
-		let retry_strategy = FibonacciBackoff::from_millis(10).take(10);
+		let retry_strategy = FibonacciBackoff::from_millis(1000).take(10);
 		let retry_future = Retry::spawn(core.handle(), retry_strategy, action);
 		let retry_result = core.run(retry_future);
 
@@ -555,11 +554,12 @@ impl WalletData {
 
 		// We successfully acquired the lock - so do what needs to be done.
 		let mut wdat = WalletData::read_or_create(data_file_path)?;
+		wdat.write(backup_file_path)?;
 		let res = f(&mut wdat);
 		wdat.write(data_file_path)?;
 
 		// delete the lock file
-		fs::remove_file(lock_file_path).context(ErrorKind::WalletData(
+		fs::remove_dir(lock_file_path).context(ErrorKind::WalletData(
 			"Could not remove wallet lock file. Maybe insufficient rights?",
 		))?;