Skip to content

Commit cabb2e5

Browse files
committed
refactor: introduce a more general LockDirectories for init
No functional change. This is in preparation for adding additional directory locks on startup.
1 parent 1db331b commit cabb2e5

File tree

5 files changed

+25
-22
lines changed

5 files changed

+25
-22
lines changed

src/bitcoind.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,10 +228,10 @@ static bool AppInit(NodeContext& node)
228228
return InitError(Untranslated("-daemon is not supported on this operating system"));
229229
#endif // HAVE_DECL_FORK
230230
}
231-
// Lock data directory after daemonization
232-
if (!AppInitLockDataDirectory())
231+
// Lock critical directories after daemonization
232+
if (!AppInitLockDirectories())
233233
{
234-
// If locking the data directory failed, exit immediately
234+
// If locking a directory failed, exit immediately
235235
return false;
236236
}
237237
fRet = AppInitInterfaces(node) && AppInitMain(node);

src/init.cpp

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,19 +1072,22 @@ bool AppInitParameterInteraction(const ArgsManager& args)
10721072
return true;
10731073
}
10741074

1075-
static bool LockDataDirectory(bool probeOnly)
1075+
static bool LockDirectory(const fs::path& dir, bool probeOnly)
10761076
{
1077-
// Make sure only a single Bitcoin process is using the data directory.
1078-
const fs::path& datadir = gArgs.GetDataDirNet();
1079-
switch (util::LockDirectory(datadir, ".lock", probeOnly)) {
1077+
// Make sure only a single process is using the directory.
1078+
switch (util::LockDirectory(dir, ".lock", probeOnly)) {
10801079
case util::LockResult::ErrorWrite:
1081-
return InitError(strprintf(_("Cannot write to data directory '%s'; check permissions."), fs::PathToString(datadir)));
1080+
return InitError(strprintf(_("Cannot write to directory '%s'; check permissions."), fs::PathToString(dir)));
10821081
case util::LockResult::ErrorLock:
1083-
return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), fs::PathToString(datadir), CLIENT_NAME));
1082+
return InitError(strprintf(_("Cannot obtain a lock on directory %s. %s is probably already running."), fs::PathToString(dir), CLIENT_NAME));
10841083
case util::LockResult::Success: return true;
10851084
} // no default case, so the compiler can warn about missing cases
10861085
assert(false);
10871086
}
1087+
static bool LockDirectories(bool probeOnly)
1088+
{
1089+
return LockDirectory(gArgs.GetDataDirNet(), probeOnly);
1090+
}
10881091

10891092
bool AppInitSanityChecks(const kernel::Context& kernel)
10901093
{
@@ -1099,19 +1102,19 @@ bool AppInitSanityChecks(const kernel::Context& kernel)
10991102
return InitError(strprintf(_("Elliptic curve cryptography sanity check failure. %s is shutting down."), CLIENT_NAME));
11001103
}
11011104

1102-
// Probe the data directory lock to give an early error message, if possible
1103-
// We cannot hold the data directory lock here, as the forking for daemon() hasn't yet happened,
1104-
// and a fork will cause weird behavior to it.
1105-
return LockDataDirectory(true);
1105+
// Probe the directory locks to give an early error message, if possible
1106+
// We cannot hold the directory locks here, as the forking for daemon() hasn't yet happened,
1107+
// and a fork will cause weird behavior to them.
1108+
return LockDirectories(true);
11061109
}
11071110

1108-
bool AppInitLockDataDirectory()
1111+
bool AppInitLockDirectories()
11091112
{
1110-
// After daemonization get the data directory lock again and hold on to it until exit
1113+
// After daemonization get the directory locks again and hold on to them until exit
11111114
// This creates a slight window for a race condition to happen, however this condition is harmless: it
11121115
// will at most make us exit without printing a message to console.
1113-
if (!LockDataDirectory(false)) {
1114-
// Detailed error printed inside LockDataDirectory
1116+
if (!LockDirectories(false)) {
1117+
// Detailed error printed inside LockDirectory
11151118
return false;
11161119
}
11171120
return true;

src/init.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,19 @@ bool AppInitParameterInteraction(const ArgsManager& args);
5555
*/
5656
bool AppInitSanityChecks(const kernel::Context& kernel);
5757
/**
58-
* Lock bitcoin core data directory.
58+
* Lock bitcoin core critical directories.
5959
* @note This should only be done after daemonization. Do not call Shutdown() if this function fails.
6060
* @pre Parameters should be parsed and config file should be read, AppInitSanityChecks should have been called.
6161
*/
62-
bool AppInitLockDataDirectory();
62+
bool AppInitLockDirectories();
6363
/**
6464
* Initialize node and wallet interface pointers. Has no prerequisites or side effects besides allocating memory.
6565
*/
6666
bool AppInitInterfaces(node::NodeContext& node);
6767
/**
6868
* Bitcoin core main initialization.
6969
* @note This should only be done after daemonization. Call Shutdown() if this function fails.
70-
* @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called.
70+
* @pre Parameters should be parsed and config file should be read, AppInitLockDirectories should have been called.
7171
*/
7272
bool AppInitMain(node::NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);
7373

src/node/interfaces.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ class NodeImpl : public Node
116116
m_context->ecc_context = std::make_unique<ECC_Context>();
117117
if (!AppInitSanityChecks(*m_context->kernel)) return false;
118118

119-
if (!AppInitLockDataDirectory()) return false;
119+
if (!AppInitLockDirectories()) return false;
120120
if (!AppInitInterfaces(*m_context)) return false;
121121

122122
return true;

test/functional/feature_filelock.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def run_test(self):
3030
self.log.info(f"Using datadir {datadir}")
3131

3232
self.log.info("Check that we can't start a second bitcoind instance using the same datadir")
33-
expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['CLIENT_NAME']} is probably already running."
33+
expected_msg = f"Error: Cannot obtain a lock on directory {datadir}. {self.config['environment']['CLIENT_NAME']} is probably already running."
3434
self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir_path}', '-noserver'], expected_msg=expected_msg)
3535

3636
self.log.info("Check that cookie and PID file are not deleted when attempting to start a second bitcoind using the same datadir")

0 commit comments

Comments
 (0)