Skip to content

Commit

Permalink
init: don't delete PID file if it was not generated
Browse files Browse the repository at this point in the history
Previously, starting a second bitcoind using the same datadir would
correctly fail to init and shutdown. However during shutdown the PID
file belonging to the first instance would be erroneously removed by
the second process shutting down.

Fix this to only delete the PID file if we created it.
  • Loading branch information
willcl-ark authored and lateminer committed May 15, 2024
1 parent 77210e4 commit 9248383
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 7 deletions.
25 changes: 18 additions & 7 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ static const char* DEFAULT_ASMAP_FILENAME="ip_asn.map";
* The PID file facilities.
*/
static const char* BITCOIN_PID_FILENAME = "blackmored.pid";
/**
* True if this process has created a PID file.
* Used to determine whether we should remove the PID file on shutdown.
*/
static bool g_generated_pid{false};

static std::shared_ptr<CWallet> walletTmp;

Expand All @@ -177,12 +182,24 @@ static fs::path GetPidFile(const ArgsManager& args)
#else
tfm::format(file, "%d\n", getpid());
#endif
g_generated_pid = true;
return true;
} else {
return InitError(strprintf(_("Unable to create the PID file '%s': %s"), fs::PathToString(GetPidFile(args)), SysErrorString(errno)));
}
}

static void RemovePidFile(const ArgsManager& args)
{
if (!g_generated_pid) return;
const auto pid_path{GetPidFile(args)};
if (std::error_code error; !fs::remove(pid_path, error)) {
std::string msg{error ? error.message() : "File does not exist"};
LogPrintf("Unable to remove PID file (%s): %s\n", fs::PathToString(pid_path), msg);
}
}


//////////////////////////////////////////////////////////////////////////////
//
// Shutdown
Expand Down Expand Up @@ -362,13 +379,7 @@ void Shutdown(NodeContext& node)
node.chainman.reset();
node.scheduler.reset();

try {
if (!fs::remove(GetPidFile(*node.args))) {
LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__);
}
} catch (const fs::filesystem_error& e) {
LogPrintf("%s: Unable to remove PID file: %s\n", __func__, fsbridge::get_filesystem_error_message(e));
}
RemovePidFile(*node.args);

LogPrintf("%s: done\n", __func__);
}
Expand Down
3 changes: 3 additions & 0 deletions test/functional/feature_filelock.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ def run_test(self):
expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_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")
cookie_file = datadir / ".cookie"
assert cookie_file.exists() # should not be deleted during the second bitcoind instance shutdown
pid_file = datadir / "blackmored.pid"
assert pid_file.exists()

if self.is_wallet_compiled():
def check_wallet_filelock(descriptors):
Expand Down

0 comments on commit 9248383

Please sign in to comment.