diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index d210e2c8ba..ceb3c99410 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -228,10 +228,10 @@ static bool AppInit(NodeContext& node) return InitError(Untranslated("-daemon is not supported on this operating system")); #endif // HAVE_DECL_FORK } - // Lock data directory after daemonization - if (!AppInitLockDataDirectory()) + // Lock critical directories after daemonization + if (!AppInitLockDirectories()) { - // If locking the data directory failed, exit immediately + // If locking a directory failed, exit immediately return false; } fRet = AppInitInterfaces(node) && AppInitMain(node); diff --git a/src/init.cpp b/src/init.cpp index 1f597cb7cb..09d9de4edc 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1072,19 +1072,23 @@ bool AppInitParameterInteraction(const ArgsManager& args) return true; } -static bool LockDataDirectory(bool probeOnly) +static bool LockDirectory(const fs::path& dir, bool probeOnly) { - // Make sure only a single Bitcoin process is using the data directory. - const fs::path& datadir = gArgs.GetDataDirNet(); - switch (util::LockDirectory(datadir, ".lock", probeOnly)) { + // Make sure only a single process is using the directory. + switch (util::LockDirectory(dir, ".lock", probeOnly)) { case util::LockResult::ErrorWrite: - return InitError(strprintf(_("Cannot write to data directory '%s'; check permissions."), fs::PathToString(datadir))); + return InitError(strprintf(_("Cannot write to directory '%s'; check permissions."), fs::PathToString(dir))); case util::LockResult::ErrorLock: - return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), fs::PathToString(datadir), CLIENT_NAME)); + return InitError(strprintf(_("Cannot obtain a lock on directory %s. %s is probably already running."), fs::PathToString(dir), CLIENT_NAME)); case util::LockResult::Success: return true; } // no default case, so the compiler can warn about missing cases assert(false); } +static bool LockDirectories(bool probeOnly) +{ + return LockDirectory(gArgs.GetDataDirNet(), probeOnly) && \ + LockDirectory(gArgs.GetBlocksDirPath(), probeOnly); +} bool AppInitSanityChecks(const kernel::Context& kernel) { @@ -1099,19 +1103,19 @@ bool AppInitSanityChecks(const kernel::Context& kernel) return InitError(strprintf(_("Elliptic curve cryptography sanity check failure. %s is shutting down."), CLIENT_NAME)); } - // Probe the data directory lock to give an early error message, if possible - // We cannot hold the data directory lock here, as the forking for daemon() hasn't yet happened, - // and a fork will cause weird behavior to it. - return LockDataDirectory(true); + // Probe the directory locks to give an early error message, if possible + // We cannot hold the directory locks here, as the forking for daemon() hasn't yet happened, + // and a fork will cause weird behavior to them. + return LockDirectories(true); } -bool AppInitLockDataDirectory() +bool AppInitLockDirectories() { - // After daemonization get the data directory lock again and hold on to it until exit + // After daemonization get the directory locks again and hold on to them until exit // This creates a slight window for a race condition to happen, however this condition is harmless: it // will at most make us exit without printing a message to console. - if (!LockDataDirectory(false)) { - // Detailed error printed inside LockDataDirectory + if (!LockDirectories(false)) { + // Detailed error printed inside LockDirectory return false; } return true; diff --git a/src/init.h b/src/init.h index 6d8a35d80e..6b60a4e147 100644 --- a/src/init.h +++ b/src/init.h @@ -55,11 +55,11 @@ bool AppInitParameterInteraction(const ArgsManager& args); */ bool AppInitSanityChecks(const kernel::Context& kernel); /** - * Lock bitcoin core data directory. + * Lock bitcoin core critical directories. * @note This should only be done after daemonization. Do not call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, AppInitSanityChecks should have been called. */ -bool AppInitLockDataDirectory(); +bool AppInitLockDirectories(); /** * Initialize node and wallet interface pointers. Has no prerequisites or side effects besides allocating memory. */ @@ -67,7 +67,7 @@ bool AppInitInterfaces(node::NodeContext& node); /** * Bitcoin core main initialization. * @note This should only be done after daemonization. Call Shutdown() if this function fails. - * @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called. + * @pre Parameters should be parsed and config file should be read, AppInitLockDirectories should have been called. */ bool AppInitMain(node::NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr); diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 57c82e5d5f..6f1a96c827 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -1117,7 +1117,19 @@ static auto InitBlocksdirXorKey(const BlockManager::Options& opts) // size of the XOR-key file. std::array xor_key{}; - if (opts.use_xor && fs::is_empty(opts.blocks_dir)) { + // Consider this to be the first run if the blocksdir contains only hidden + // files (those which start with a .). Checking for a fully-empty dir would + // be too aggressive as a .lock file may have already been written. + bool first_run = true; + for (const auto& entry : fs::directory_iterator(opts.blocks_dir)) { + const std::string path = fs::PathToString(entry.path().filename()); + if (!entry.is_regular_file() || !path.starts_with('.')) { + first_run = false; + break; + } + } + + if (opts.use_xor && first_run) { // Only use random fresh key when the boolean option is set and on the // very first start of the program. FastRandomContext{}.fillrand(xor_key); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index fe88a6dad1..7ae2ff6453 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -116,7 +116,7 @@ public: m_context->ecc_context = std::make_unique(); if (!AppInitSanityChecks(*m_context->kernel)) return false; - if (!AppInitLockDataDirectory()) return false; + if (!AppInitLockDirectories()) return false; if (!AppInitInterfaces(*m_context)) return false; return true; diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py index f56643c62e..aa4ca4b45a 100755 --- a/test/functional/feature_filelock.py +++ b/test/functional/feature_filelock.py @@ -27,13 +27,19 @@ class FilelockTest(BitcoinTestFramework): def run_test(self): datadir = self.nodes[0].chain_path + blocksdir = self.nodes[0].blocks_path self.log.info(f"Using datadir {datadir}") + self.log.info(f"Using blocksdir {blocksdir}") self.log.info("Check that we can't start a second bitcoind instance using the same datadir") - expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['CLIENT_NAME']} is probably already running." + expected_msg = f"Error: Cannot obtain a lock on directory {datadir}. {self.config['environment']['CLIENT_NAME']} is probably already running." self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir_path}', '-noserver'], expected_msg=expected_msg) - self.log.info("Check that cookie and PID file are not deleted when attempting to start a second bitcoind using the same datadir") + self.log.info("Check that we can't start a second bitcoind instance using the same blocksdir") + expected_msg = f"Error: Cannot obtain a lock on directory {blocksdir}. {self.config['environment']['CLIENT_NAME']} is probably already running." + self.nodes[1].assert_start_raises_init_error(extra_args=[f'-blocksdir={self.nodes[0].datadir_path}', '-noserver'], expected_msg=expected_msg) + + self.log.info("Check that cookie and PID file are not deleted when attempting to start a second bitcoind using the same datadir/blocksdir") cookie_file = datadir / ".cookie" assert cookie_file.exists() # should not be deleted during the second bitcoind instance shutdown pid_file = datadir / BITCOIN_PID_FILENAME_DEFAULT