diff options
author | Wladimir J. van der Laan <laanwj@gmail.com> | 2018-01-16 10:54:13 +0100 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2018-01-16 11:11:59 +0100 |
commit | 66e3af709dd444b2d85e15c56f4608c700ff82ee (patch) | |
tree | a604dd3608644a7cc968253bde02cbedbd30d916 /src | |
parent | bbc91b7699732efc20ac1526383515c944b66d70 (diff) | |
parent | 2f3bd47d44634cfc0a4261e64af178407ce2869c (diff) | |
download | bitcoin-66e3af709dd444b2d85e15c56f4608c700ff82ee.tar.xz |
Merge #11904: Add a lock to the wallet directory
2f3bd47 Abstract directory locking into util.cpp (MeshCollider)
5260a4a Make .walletlock distinct from .lock (MeshCollider)
64226de Generalise walletdir lock error message for correctness (MeshCollider)
c9ed4bd Add a test for wallet directory locking (MeshCollider)
e60cb99 Add a lock to the wallet directory (MeshCollider)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/11888, needs a 0.16 milestone
Also adds a test that the lock works.
https://github.com/bitcoin/bitcoin/pull/11687 will probably rework this to a per-wallet lock instead of just the walletdir, but this fixes the current issue
Tree-SHA512: 98e52d67f820e3b8f919cf361ffbb7d928f1bd67603e0ed26c5076ea02d9b3a90c3535ddf7329f3b88171396fa28dd3c87adab3577a8a217bd1e4247bda99138
Diffstat (limited to 'src')
-rw-r--r-- | src/init.cpp | 19 | ||||
-rw-r--r-- | src/util.cpp | 22 | ||||
-rw-r--r-- | src/util.h | 1 | ||||
-rw-r--r-- | src/wallet/db.cpp | 48 | ||||
-rw-r--r-- | src/wallet/db.h | 2 |
5 files changed, 55 insertions, 37 deletions
diff --git a/src/init.cpp b/src/init.cpp index 7215e87359..b48802637b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1143,23 +1143,10 @@ bool AppInitParameterInteraction() static bool LockDataDirectory(bool probeOnly) { - std::string strDataDir = GetDataDir().string(); - // Make sure only a single Bitcoin process is using the data directory. - fs::path pathLockFile = GetDataDir() / ".lock"; - FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist. - if (file) fclose(file); - - try { - static boost::interprocess::file_lock lock(pathLockFile.string().c_str()); - if (!lock.try_lock()) { - return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), strDataDir, _(PACKAGE_NAME))); - } - if (probeOnly) { - lock.unlock(); - } - } catch(const boost::interprocess::interprocess_exception& e) { - return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running.") + " %s.", strDataDir, _(PACKAGE_NAME), e.what())); + fs::path datadir = GetDataDir(); + if (!LockDirectory(datadir, ".lock", probeOnly)) { + return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), datadir.string(), _(PACKAGE_NAME))); } return true; } diff --git a/src/util.cpp b/src/util.cpp index 150bc503df..80eed24ffd 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -72,6 +72,7 @@ #include <boost/algorithm/string/case_conv.hpp> // for to_lower() #include <boost/algorithm/string/predicate.hpp> // for startswith() and endswith() +#include <boost/interprocess/sync/file_lock.hpp> #include <boost/program_options/detail/config_file.hpp> #include <boost/thread.hpp> #include <openssl/crypto.h> @@ -375,6 +376,27 @@ int LogPrintStr(const std::string &str) return ret; } +bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only) +{ + fs::path pathLockFile = directory / lockfile_name; + FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist. + if (file) fclose(file); + + try { + static std::map<std::string, boost::interprocess::file_lock> locks; + boost::interprocess::file_lock& lock = locks.emplace(pathLockFile.string(), pathLockFile.string().c_str()).first->second; + if (!lock.try_lock()) { + return false; + } + if (probe_only) { + lock.unlock(); + } + } catch (const boost::interprocess::interprocess_exception& e) { + return error("Error while attempting to lock directory %s: %s", directory.string(), e.what()); + } + return true; +} + /** Interpret string as boolean, for argument parsing */ static bool InterpretBool(const std::string& strValue) { diff --git a/src/util.h b/src/util.h index 6a0d6a31e7..277b4c66af 100644 --- a/src/util.h +++ b/src/util.h @@ -173,6 +173,7 @@ bool TruncateFile(FILE *file, unsigned int length); int RaiseFileDescriptorLimit(int nMinFD); void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length); bool RenameOver(fs::path src, fs::path dest); +bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only=false); bool TryCreateDirectories(const fs::path& p); fs::path GetDefaultDataDir(); const fs::path &GetDataDir(bool fNetSpecific = true); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 35ff0e1eec..23c6279128 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -95,7 +95,7 @@ void CDBEnv::Close() EnvShutdown(); } -bool CDBEnv::Open(const fs::path& pathIn) +bool CDBEnv::Open(const fs::path& pathIn, bool retry) { if (fDbEnvInit) return true; @@ -103,6 +103,11 @@ bool CDBEnv::Open(const fs::path& pathIn) boost::this_thread::interruption_point(); strPath = pathIn.string(); + if (!LockDirectory(pathIn, ".walletlock")) { + LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it.\n", strPath); + return false; + } + fs::path pathLogDir = pathIn / "database"; TryCreateDirectories(pathLogDir); fs::path pathErrorFile = pathIn / "db.log"; @@ -134,7 +139,24 @@ bool CDBEnv::Open(const fs::path& pathIn) S_IRUSR | S_IWUSR); if (ret != 0) { dbenv->close(0); - return error("CDBEnv::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); + LogPrintf("CDBEnv::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); + if (retry) { + // try moving the database env out of the way + fs::path pathDatabaseBak = pathIn / strprintf("database.%d.bak", GetTime()); + try { + fs::rename(pathLogDir, pathDatabaseBak); + LogPrintf("Moved old %s to %s. Retrying.\n", pathLogDir.string(), pathDatabaseBak.string()); + } catch (const fs::filesystem_error&) { + // failure is ok (well, not really, but it's not worse than what we started with) + } + // try opening it again one more time + if (!Open(pathIn, false)) { + // if it still fails, it probably means we can't even create the database env + return false; + } + } else { + return false; + } } fDbEnvInit = true; @@ -269,25 +291,11 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle return false; } - if (!bitdb.Open(walletDir)) - { - // try moving the database env out of the way - fs::path pathDatabase = walletDir / "database"; - fs::path pathDatabaseBak = walletDir / strprintf("database.%d.bak", GetTime()); - try { - fs::rename(pathDatabase, pathDatabaseBak); - LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string()); - } catch (const fs::filesystem_error&) { - // failure is ok (well, not really, but it's not worse than what we started with) - } - - // try again - if (!bitdb.Open(walletDir)) { - // if it still fails, it probably means we can't even create the database env - errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); - return false; - } + if (!bitdb.Open(walletDir, true)) { + errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); + return false; } + return true; } diff --git a/src/wallet/db.h b/src/wallet/db.h index c6f317927f..787135e400 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -68,7 +68,7 @@ public: typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair; bool Salvage(const std::string& strFile, bool fAggressive, std::vector<KeyValPair>& vResult); - bool Open(const fs::path& path); + bool Open(const fs::path& path, bool retry = 0); void Close(); void Flush(bool fShutdown); void CheckpointLSN(const std::string& strFile); |