From 7d61e03c701215e2956353195dff4734210a2765 Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Wed, 4 Mar 2026 11:58:12 -0800 Subject: [PATCH] args: extract lock-requiring internal helpers Extract GetSetting_(), GetArgFlags_(), and GetPathArg_() as private lock-held helpers with EXCLUSIVE_LOCKS_REQUIRED(cs_args) annotations. Public methods delegate to these after acquiring the lock. Annotate GetDataDir() with EXCLUSIVE_LOCKS_REQUIRED(cs_args) and move its lock acquisition into GetDataDirBase()/GetDataDirNet(). Annotate logArgsPrefix() with EXCLUSIVE_LOCKS_REQUIRED(cs_args). This is a pure refactoring with no behavior change, preparing for the conversion of cs_args from RecursiveMutex to Mutex. --- src/common/args.cpp | 52 ++++++++++++++++++++++++++++++++++++--------- src/common/args.h | 13 ++++++++---- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/common/args.cpp b/src/common/args.cpp index 5c8589cf440..a460c0bdfc0 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -255,9 +255,9 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin return true; } -std::optional ArgsManager::GetArgFlags(const std::string& name) const +std::optional ArgsManager::GetArgFlags_(const std::string& name) const { - LOCK(cs_args); + AssertLockHeld(cs_args); for (const auto& arg_map : m_available_args) { const auto search = arg_map.second.find(name); if (search != arg_map.second.end()) { @@ -267,22 +267,36 @@ std::optional ArgsManager::GetArgFlags(const std::string& name) co return m_default_flags; } +std::optional ArgsManager::GetArgFlags(const std::string& name) const +{ + LOCK(cs_args); + return GetArgFlags_(name); +} + void ArgsManager::SetDefaultFlags(std::optional flags) { LOCK(cs_args); m_default_flags = flags; } -fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value) const +fs::path ArgsManager::GetPathArg_(std::string arg, const fs::path& default_value) const { - if (IsArgNegated(arg)) return fs::path{}; - std::string path_str = GetArg(arg, ""); + AssertLockHeld(cs_args); + const auto value = GetSetting_(arg); + if (value.isFalse()) return {}; + std::string path_str = SettingToString(value, ""); if (path_str.empty()) return default_value; fs::path result = fs::PathFromString(path_str).lexically_normal(); // Remove trailing slash, if present. return result.has_filename() ? result : result.parent_path(); } +fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value) const +{ + LOCK(cs_args); + return GetPathArg_(std::move(arg), default_value); +} + fs::path ArgsManager::GetBlocksDirPath() const { LOCK(cs_args); @@ -308,15 +322,25 @@ fs::path ArgsManager::GetBlocksDirPath() const return path; } +fs::path ArgsManager::GetDataDirBase() const { + LOCK(cs_args); + return GetDataDir(/*net_specific=*/false); +} + +fs::path ArgsManager::GetDataDirNet() const { + LOCK(cs_args); + return GetDataDir(/*net_specific=*/true); +} + fs::path ArgsManager::GetDataDir(bool net_specific) const { - LOCK(cs_args); + AssertLockHeld(cs_args); fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path; // Used cached path if available if (!path.empty()) return path; - const fs::path datadir{GetPathArg("-datadir")}; + const fs::path datadir{GetPathArg_("-datadir")}; if (!datadir.empty()) { path = fs::absolute(datadir); if (!fs::is_directory(path)) { @@ -853,15 +877,22 @@ std::variant ArgsManager::GetChainArg() const bool ArgsManager::UseDefaultSection(const std::string& arg) const { + AssertLockHeld(cs_args); return m_network == ChainTypeToString(ChainType::MAIN) || !m_network_only_args.contains(arg); } +common::SettingsValue ArgsManager::GetSetting_(const std::string& arg) const +{ + AssertLockHeld(cs_args); + return common::GetSetting( + m_settings, m_network, SettingName(arg), !UseDefaultSection(arg), + /*ignore_nonpersistent=*/false, /*get_chain_type=*/false); +} + common::SettingsValue ArgsManager::GetSetting(const std::string& arg) const { LOCK(cs_args); - return common::GetSetting( - m_settings, m_network, SettingName(arg), !UseDefaultSection(arg), - /*ignore_nonpersistent=*/false, /*get_chain_type=*/false); + return GetSetting_(arg); } std::vector ArgsManager::GetSettingsList(const std::string& arg) const @@ -875,6 +906,7 @@ void ArgsManager::logArgsPrefix( const std::string& section, const std::map>& args) const { + AssertLockHeld(cs_args); std::string section_str = section.empty() ? "" : "[" + section + "] "; for (const auto& arg : args) { for (const auto& value : arg.second) { diff --git a/src/common/args.h b/src/common/args.h index ea4e173bf72..0bcf32d1f88 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -229,14 +229,14 @@ protected: * * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned */ - fs::path GetDataDirBase() const { return GetDataDir(false); } + fs::path GetDataDirBase() const; /** * Get data directory path with appended network identifier * * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned */ - fs::path GetDataDirNet() const { return GetDataDir(true); } + fs::path GetDataDirNet() const; /** * Clear cached directory paths @@ -435,13 +435,18 @@ protected: void LogArgs() const; private: + // Internal helpers, for use by callers that already hold `cs_args`. + common::SettingsValue GetSetting_(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(cs_args); + std::optional GetArgFlags_(const std::string& name) const EXCLUSIVE_LOCKS_REQUIRED(cs_args); + fs::path GetPathArg_(std::string arg, const fs::path& default_value = {}) const EXCLUSIVE_LOCKS_REQUIRED(cs_args); + /** * Get data directory path * * @param net_specific Append network identifier to the returned path * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned */ - fs::path GetDataDir(bool net_specific) const; + fs::path GetDataDir(bool net_specific) const EXCLUSIVE_LOCKS_REQUIRED(cs_args); /** * Return -regtest/-signet/-testnet/-testnet4/-chain= setting as a ChainType enum if a @@ -455,7 +460,7 @@ private: void logArgsPrefix( const std::string& prefix, const std::string& section, - const std::map>& args) const; + const std::map>& args) const EXCLUSIVE_LOCKS_REQUIRED(cs_args); }; extern ArgsManager gArgs;