diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -1704,33 +1704,12 @@ } 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"; - // empty lock file; created if it doesn't exist. - FILE *file = fsbridge::fopen(pathLockFile, "a"); - 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) { + 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.") + - " %s.", - strDataDir, _(PACKAGE_NAME), e.what())); + "%s. %s is probably already running."), + datadir.string(), _(PACKAGE_NAME))); } return true; } diff --git a/src/util.h b/src/util.h --- a/src/util.h +++ b/src/util.h @@ -72,6 +72,8 @@ 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/util.cpp b/src/util.cpp --- a/src/util.cpp +++ b/src/util.cpp @@ -76,6 +76,7 @@ #include #endif +#include #include #include @@ -144,6 +145,33 @@ } } instance_of_cinit; +bool LockDirectory(const fs::path &directory, const std::string lockfile_name, + bool probe_only) { + fs::path pathLockFile = directory / lockfile_name; + // empty lock file; created if it doesn't exist. + FILE *file = fsbridge::fopen(pathLockFile, "a"); + if (file) { + fclose(file); + } + + try { + static std::map 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 a string argument as a boolean. * diff --git a/src/wallet/db.h b/src/wallet/db.h --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -72,7 +72,7 @@ bool Salvage(const std::string &strFile, bool fAggressive, std::vector &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); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -104,7 +104,7 @@ EnvShutdown(); } -bool CDBEnv::Open(const fs::path &pathIn) { +bool CDBEnv::Open(const fs::path &pathIn, bool retry) { if (fDbEnvInit) { return true; } @@ -112,6 +112,13 @@ 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"; @@ -142,9 +149,29 @@ 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; @@ -279,30 +306,12 @@ 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/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -16,9 +16,9 @@ class MultiWalletTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 1 + self.num_nodes = 2 self.extra_args = [ - ['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w']] + ['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w'], []] self.supports_cli = True def run_test(self): @@ -31,7 +31,7 @@ assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"}) - self.stop_node(0) + self.stop_nodes() self.assert_start_raises_init_error( 0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') @@ -75,20 +75,24 @@ assert_equal(set(node.listwallets()), {"w4", "w5"}) w5 = wallet("w5") w5.generate(1) - self.stop_node(0) # now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded os.rename(wallet_dir2, wallet_dir()) - self.start_node(0, ['-wallet=w4', '-wallet=w5', - '-walletdir=' + data_dir()]) + self.restart_node(0, ['-wallet=w4', '-wallet=w5', + '-walletdir=' + data_dir()]) assert_equal(set(node.listwallets()), {"w4", "w5"}) w5 = wallet("w5") w5_info = w5.getwalletinfo() assert_equal(w5_info['immature_balance'], 50) - self.stop_node(0) + competing_wallet_dir = os.path.join( + self.options.tmpdir, 'competing_walletdir') + os.mkdir(competing_wallet_dir) + self.restart_node(0, ['-walletdir='+competing_wallet_dir]) + self.assert_start_raises_init_error( + 1, ['-walletdir='+competing_wallet_dir], 'Error initializing wallet database environment') - self.start_node(0, self.extra_args[0]) + self.restart_node(0, self.extra_args[0]) w1 = wallet("w1") w2 = wallet("w2")