refactor: introduce a more general LockDirectories for init

No functional change. This is in preparation for adding additional directory
locks on startup.
This commit is contained in:
Cory Fields 2025-01-15 18:03:43 +00:00
parent 1db331ba76
commit cabb2e5c24
5 changed files with 25 additions and 22 deletions

View file

@ -228,10 +228,10 @@ static bool AppInit(NodeContext& node)
return InitError(Untranslated("-daemon is not supported on this operating system")); return InitError(Untranslated("-daemon is not supported on this operating system"));
#endif // HAVE_DECL_FORK #endif // HAVE_DECL_FORK
} }
// Lock data directory after daemonization // Lock critical directories after daemonization
if (!AppInitLockDataDirectory()) if (!AppInitLockDirectories())
{ {
// If locking the data directory failed, exit immediately // If locking a directory failed, exit immediately
return false; return false;
} }
fRet = AppInitInterfaces(node) && AppInitMain(node); fRet = AppInitInterfaces(node) && AppInitMain(node);

View file

@ -1072,19 +1072,22 @@ bool AppInitParameterInteraction(const ArgsManager& args)
return true; 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. // Make sure only a single process is using the directory.
const fs::path& datadir = gArgs.GetDataDirNet(); switch (util::LockDirectory(dir, ".lock", probeOnly)) {
switch (util::LockDirectory(datadir, ".lock", probeOnly)) {
case util::LockResult::ErrorWrite: 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: 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; case util::LockResult::Success: return true;
} // no default case, so the compiler can warn about missing cases } // no default case, so the compiler can warn about missing cases
assert(false); assert(false);
} }
static bool LockDirectories(bool probeOnly)
{
return LockDirectory(gArgs.GetDataDirNet(), probeOnly);
}
bool AppInitSanityChecks(const kernel::Context& kernel) bool AppInitSanityChecks(const kernel::Context& kernel)
{ {
@ -1099,19 +1102,19 @@ bool AppInitSanityChecks(const kernel::Context& kernel)
return InitError(strprintf(_("Elliptic curve cryptography sanity check failure. %s is shutting down."), CLIENT_NAME)); 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 // Probe the directory locks 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, // We cannot hold the directory locks here, as the forking for daemon() hasn't yet happened,
// and a fork will cause weird behavior to it. // and a fork will cause weird behavior to them.
return LockDataDirectory(true); 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 // 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. // will at most make us exit without printing a message to console.
if (!LockDataDirectory(false)) { if (!LockDirectories(false)) {
// Detailed error printed inside LockDataDirectory // Detailed error printed inside LockDirectory
return false; return false;
} }
return true; return true;

View file

@ -55,11 +55,11 @@ bool AppInitParameterInteraction(const ArgsManager& args);
*/ */
bool AppInitSanityChecks(const kernel::Context& kernel); 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. * @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. * @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. * 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. * Bitcoin core main initialization.
* @note This should only be done after daemonization. Call Shutdown() if this function fails. * @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); bool AppInitMain(node::NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);

View file

@ -116,7 +116,7 @@ public:
m_context->ecc_context = std::make_unique<ECC_Context>(); m_context->ecc_context = std::make_unique<ECC_Context>();
if (!AppInitSanityChecks(*m_context->kernel)) return false; if (!AppInitSanityChecks(*m_context->kernel)) return false;
if (!AppInitLockDataDirectory()) return false; if (!AppInitLockDirectories()) return false;
if (!AppInitInterfaces(*m_context)) return false; if (!AppInitInterfaces(*m_context)) return false;
return true; return true;

View file

@ -30,7 +30,7 @@ class FilelockTest(BitcoinTestFramework):
self.log.info(f"Using datadir {datadir}") self.log.info(f"Using datadir {datadir}")
self.log.info("Check that we can't start a second bitcoind instance using the same datadir") 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.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 cookie and PID file are not deleted when attempting to start a second bitcoind using the same datadir")