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.
This commit is contained in:
Severus Sneep 2018-04-24 22:03:01 +02:00 committed by Ignotus Peverell
parent e22d025dc8
commit 8227ce941a

View file

@ -18,7 +18,7 @@ use std::fmt;
use std::fmt::Display; use std::fmt::Display;
use uuid::Uuid; use uuid::Uuid;
use std::convert::From; use std::convert::From;
use std::fs::{self, File, OpenOptions}; use std::fs::{self, File};
use std::io::{Read, Write}; use std::io::{Read, Write};
use std::path::Path; use std::path::Path;
use std::path::MAIN_SEPARATOR; use std::path::MAIN_SEPARATOR;
@ -47,6 +47,7 @@ use util::secp::key::PublicKey;
use util::LOGGER; use util::LOGGER;
const DAT_FILE: &'static str = "wallet.dat"; const DAT_FILE: &'static str = "wallet.dat";
const BCK_FILE: &'static str = "wallet.bck";
const LOCK_FILE: &'static str = "wallet.lock"; const LOCK_FILE: &'static str = "wallet.lock";
const SEED_FILE: &'static str = "wallet.seed"; 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 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); let lock_file_path = &format!("{}{}{}", data_file_dir, MAIN_SEPARATOR, LOCK_FILE);
info!(LOGGER, "Acquiring wallet lock ..."); info!(LOGGER, "Acquiring wallet lock ...");
let action = || { let action = || {
trace!(LOGGER, "making lock file for wallet lock"); trace!(LOGGER, "making lock file for wallet lock");
OpenOptions::new() fs::create_dir(lock_file_path)
.write(true)
.create_new(true)
.open(lock_file_path)
}; };
// use tokio_retry to cleanly define some retry logic // use tokio_retry to cleanly define some retry logic
let mut core = reactor::Core::new().unwrap(); 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_future = Retry::spawn(core.handle(), retry_strategy, action);
let retry_result = core.run(retry_future); 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. // We successfully acquired the lock - so do what needs to be done.
let mut wdat = WalletData::read_or_create(data_file_path)?; let mut wdat = WalletData::read_or_create(data_file_path)?;
wdat.write(backup_file_path)?;
let res = f(&mut wdat); let res = f(&mut wdat);
wdat.write(data_file_path)?; wdat.write(data_file_path)?;
// delete the lock file // 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?", "Could not remove wallet lock file. Maybe insufficient rights?",
))?; ))?;