diff --git a/src/init.cpp b/src/init.cpp index fae45eb90a5..97aed15c9a5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1134,11 +1134,6 @@ static bool LockDirectory(const fs::path& dir, bool probeOnly) } // 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) { @@ -1156,7 +1151,8 @@ bool AppInitSanityChecks(const kernel::Context& kernel) // 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); + return LockDirectory(gArgs.GetDataDirNet(), /*probeOnly=*/true) + && LockDirectory(gArgs.GetBlocksDirPath(), /*probeOnly=*/true); } bool AppInitLockDirectories() @@ -1164,11 +1160,7 @@ bool AppInitLockDirectories() // 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 (!LockDirectories(false)) { - // Detailed error printed inside LockDirectory - return false; - } - return true; + return LockDirectory(gArgs.GetDataDirNet(), /*probeOnly=*/false); } bool AppInitInterfaces(NodeContext& node) diff --git a/src/init/common.cpp b/src/init/common.cpp index 7191854c747..a2dde16df08 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -115,9 +115,10 @@ bool StartLogging(const ArgsManager& args) } if (!LogInstance().m_log_timestamps) - LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime())); - LogPrintf("Default data directory %s\n", fs::PathToString(GetDefaultDataDir())); - LogPrintf("Using data directory %s\n", fs::PathToString(gArgs.GetDataDirNet())); + LogInfo("Startup time: %s", FormatISO8601DateTime(GetTime())); + LogInfo("Default data directory %s", fs::PathToString(GetDefaultDataDir())); + LogInfo("Using data directory %s", fs::PathToString(gArgs.GetDataDirNet())); + LogInfo("Using blocks directory %s", fs::PathToString(gArgs.GetBlocksDirPath())); // Only log conf file usage message if conf file actually exists. fs::path config_file_path = args.GetConfigFilePath(); diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index fe44aa8a3f2..1df72853e83 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -1150,7 +1151,8 @@ static auto InitBlocksdirXorKey(const BlockManager::Options& opts) } BlockManager::BlockManager(const util::SignalInterrupt& interrupt, Options opts) - : m_prune_mode{opts.prune_target > 0}, + : m_blocks_dir_lock{DirectoryLock(opts.blocks_dir, "blocks")}, + m_prune_mode{opts.prune_target > 0}, m_xor_key{InitBlocksdirXorKey(opts)}, m_opts{std::move(opts)}, m_block_file_seq{FlatFileSeq{m_opts.blocks_dir, "blk", m_opts.fast_prune ? 0x4000 /* 16kB */ : BLOCKFILE_CHUNK_SIZE}}, diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 665c2ccd834..e9b35685efe 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -127,7 +128,6 @@ struct BlockfileCursor { std::ostream& operator<<(std::ostream& os, const BlockfileCursor& cursor); - /** * Maintains a tree of blocks (stored in `m_block_index`) which is consulted * to determine where the most-work tip is. @@ -141,6 +141,7 @@ class BlockManager friend ChainstateManager; private: + DirectoryLock m_blocks_dir_lock; const CChainParams& GetParams() const { return m_opts.chainparams; } const Consensus::Params& GetConsensus() const { return m_opts.chainparams.GetConsensus(); } /** diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 4cacbd1151f..7cc3452d936 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1244,13 +1244,13 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory) BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname), util::LockResult::Success); // Another lock on the directory from the same thread should succeed - BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname), util::LockResult::Success); + BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname), util::LockResult::ErrorLock); // Another lock on the directory from a different thread within the same process should succeed util::LockResult threadresult; std::thread thr([&] { threadresult = util::LockDirectory(dirname, lockname); }); thr.join(); - BOOST_CHECK_EQUAL(threadresult, util::LockResult::Success); + BOOST_CHECK_EQUAL(threadresult, util::LockResult::ErrorLock); #ifndef WIN32 // Try to acquire lock in child process while we're holding it, this should fail. BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1); @@ -1291,6 +1291,30 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory) BOOST_CHECK_EQUAL(processstatus, 0); BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname, true), util::LockResult::Success); + { + auto lock{DirectoryLock(dirname, "test")}; + BOOST_CHECK_THROW(DirectoryLock(dirname, "test"), std::runtime_error); + } + { + BOOST_CHECK_NO_THROW(DirectoryLock(dirname, "test")); + } + + { + DirectoryLock lock1(dirname, "test"); + DirectoryLock lock2(std::move(lock1)); + BOOST_CHECK_THROW(DirectoryLock(dirname, "test"), std::runtime_error); + } + + { + auto dirname_move = dirname / "move"; + fs::create_directories(dirname_move); + DirectoryLock lock1(dirname, "test"); + DirectoryLock lock2(dirname_move, "test"); + lock2 = std::move(lock1); + BOOST_CHECK_THROW(DirectoryLock(dirname, "test"), std::runtime_error); + BOOST_CHECK_NO_THROW(DirectoryLock(dirname_move, "test")); + } + // Restore SIGCHLD signal(SIGCHLD, old_handler); BOOST_CHECK_EQUAL(close(fd[1]), 0); // Close our side of the socketpair diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index 4d06afe1442..776c456548a 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -57,7 +58,8 @@ LockResult LockDirectory(const fs::path& directory, const fs::path& lockfile_nam // If a lock for this directory already exists in the map, don't try to re-lock it if (dir_locks.count(fs::PathToString(pathLockFile))) { - return LockResult::Success; + LogError("Error while attempting to lock directory %s: Lock already taken", fs::PathToString(directory)); + return LockResult::ErrorLock; } // Create empty lock file if it doesn't exist. @@ -90,6 +92,50 @@ void ReleaseDirectoryLocks() dir_locks.clear(); } +DirectoryLock::DirectoryLock(fs::path dir_path, std::string name) + : m_path{dir_path}, + m_name{name} +{ + // Ensures only a single lock is taken on the provided directory. + switch (util::LockDirectory(m_path, ".lock", false)) { + case util::LockResult::ErrorWrite: + throw std::runtime_error(strprintf(_("Cannot write to %s directory '%s'; check permissions."), m_name, fs::PathToString(m_path)).original); + case util::LockResult::ErrorLock: + throw std::runtime_error(strprintf(_("Cannot obtain a lock on %s directory %s. %s is probably already running."), m_name, fs::PathToString(m_path), CLIENT_NAME).original); + case util::LockResult::Success: + return; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +DirectoryLock::DirectoryLock(DirectoryLock&& other) noexcept + : m_path{std::move(other.m_path)}, + m_name{std::move(other.m_name)} +{ + other.m_path.clear(); + other.m_name.clear(); +} + +DirectoryLock& DirectoryLock::operator=(DirectoryLock&& other) noexcept +{ + if (this != &other) { + if (!m_path.empty()) { + UnlockDirectory(m_path, ".lock"); + } + m_path = std::move(other.m_path); + other.m_path.clear(); + m_name = std::move(other.m_name); + other.m_name.clear(); + } + return *this; +} + + +DirectoryLock::~DirectoryLock() +{ + if (!m_path.empty()) UnlockDirectory(m_path, ".lock"); +} + bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes) { constexpr uint64_t min_disk_space = 52428800; // 50 MiB diff --git a/src/util/fs_helpers.h b/src/util/fs_helpers.h index 28dd6d979d5..89b4078fd50 100644 --- a/src/util/fs_helpers.h +++ b/src/util/fs_helpers.h @@ -45,6 +45,23 @@ enum class LockResult { [[nodiscard]] LockResult LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only = false); } // namespace util void UnlockDirectory(const fs::path& directory, const fs::path& lockfile_name); + +class DirectoryLock +{ + fs::path m_path; + std::string m_name; + +public: + explicit DirectoryLock(fs::path dir_path, std::string name); + ~DirectoryLock(); + + DirectoryLock(const DirectoryLock&) = delete; + DirectoryLock& operator=(const DirectoryLock&) = delete; + + DirectoryLock(DirectoryLock&& other) noexcept; + DirectoryLock& operator=(DirectoryLock&& other) noexcept; +}; + bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes = 0); /** Get the size of a file by scanning it.