← Watch feed
args: replace cs_args RecursiveMutex with Mutex
Public commit record
68/100 · Adequate
What the developer wrote
Authored by w0xlt
args: replace cs_args RecursiveMutex with Mutex
Replace the RecursiveMutex with a plain Mutex now that all recursive
lock acquisitions have been eliminated in the preceding commits.
Add EXCLUSIVE_LOCKS_REQUIRED(!cs_args) negative capability annotations
to all public and protected methods that acquire cs_args, following the
pattern established in prior RecursiveMutex conversions (e.g. CAddrMan,
CBlockPolicyEstimator).
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This is a routine internal cleanup in Bitcoin Core. It swaps a recursive mutex (a lock that allows the same thread to lock it multiple times) for a plain mutex in the argument/settings manager, and adds compiler annotations so the code cannot accidentally try to hold the lock twice on the same call path. There is no user-facing bug fix or security patch here.
Recommended action
No action required. Treat as normal code maintenance. Reviewers may verify that preceding commits in the series did indeed remove all recursive `cs_args` acquisitions, otherwise a plain mutex could introduce self-deadlock.
Security signals we found
01
Mutex type conversion from RecursiveMutex to Mutex
02
Addition of EXCLUSIVE_LOCKS_REQUIRED(!cs_args) negative capability annotations
03
Refactoring-only change with no functional diff beyond type/annotation
Evidence from the diff
The commit changes cs_args from RecursiveMutex to Mutex in src/common/args.h and adds EXCLUSIVE_LOCKS_REQUIRED(!cs_args) annotations to public/protected ArgsManager methods. The annotations are negative capabilities telling the static thread-safety analyzer that callers must not already hold cs_args before calling these methods. This follows prior conversions (CAddrMan, CBlockPolicyEstimator) and relies on preceding commits that removed all recursive acquisitions of cs_args. The change is purely a refactoring/locking-hardening step; no functional behavior or vulnerability is addressed.
Changed components
src/common/args.hArgsManagercs_args synchronization primitiveInspect captured patch +48 / −48
diff --git a/src/common/args.h b/src/common/args.h
index 0bcf32d1..fa22c36d 100644
--- a/src/common/args.h
+++ b/src/common/args.h
@@ -135,7 +135,7 @@ protected:
unsigned int m_flags;
};
- mutable RecursiveMutex cs_args;
+ mutable Mutex cs_args;
common::Settings m_settings GUARDED_BY(cs_args);
std::vector<std::string> m_command GUARDED_BY(cs_args);
std::string m_network GUARDED_BY(cs_args);
@@ -149,7 +149,7 @@ protected:
mutable fs::path m_cached_datadir_path GUARDED_BY(cs_args);
mutable fs::path m_cached_network_datadir_path GUARDED_BY(cs_args);
- [[nodiscard]] bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false);
+ [[nodiscard]] bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Returns true if settings values from the default section should be used,
@@ -166,12 +166,12 @@ protected:
* false if "-nosetting" argument was passed, and a string if a "-setting=value"
* argument was passed.
*/
- common::SettingsValue GetSetting(const std::string& arg) const;
+ common::SettingsValue GetSetting(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get list of setting values.
*/
- std::vector<common::SettingsValue> GetSettingsList(const std::string& arg) const;
+ std::vector<common::SettingsValue> GetSettingsList(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
ArgsManager();
~ArgsManager();
@@ -179,16 +179,16 @@ protected:
/**
* Select the network in use
*/
- void SelectConfigNetwork(const std::string& network);
+ void SelectConfigNetwork(const std::string& network) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
- [[nodiscard]] bool ParseParameters(int argc, const char* const argv[], std::string& error);
+ [[nodiscard]] bool ParseParameters(int argc, const char* const argv[], std::string& error) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return config file path (read-only)
*/
- fs::path GetConfigFilePath() const;
- void SetConfigFilePath(fs::path);
- [[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false);
+ fs::path GetConfigFilePath() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
+ void SetConfigFilePath(fs::path) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
+ [[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Log warnings for options in m_section_only_args when
@@ -196,12 +196,12 @@ protected:
* on the command line or in a network-specific section in the
* config file.
*/
- std::set<std::string> GetUnsuitableSectionOnlyArgs() const;
+ std::set<std::string> GetUnsuitableSectionOnlyArgs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Log warnings for unrecognized section names in the config file.
*/
- std::list<SectionInfo> GetUnrecognizedSections() const;
+ std::list<SectionInfo> GetUnrecognizedSections() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
struct Command {
/** The command (if one has been registered with AddCommand), or empty */
@@ -215,33 +215,33 @@ protected:
/**
* Get the command and command args (returns std::nullopt if no command provided)
*/
- std::optional<const Command> GetCommand() const;
+ std::optional<const Command> GetCommand() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get blocks directory path
*
* @return Blocks path which is network specific
*/
- fs::path GetBlocksDirPath() const;
+ fs::path GetBlocksDirPath() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get data directory path
*
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
*/
- fs::path GetDataDirBase() const;
+ fs::path GetDataDirBase() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* 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;
+ fs::path GetDataDirNet() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Clear cached directory paths
*/
- void ClearPathCache();
+ void ClearPathCache() EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return a vector of strings of the given argument
@@ -249,7 +249,7 @@ protected:
* @param strArg Argument to get (e.g. "-foo")
* @return command-line arguments
*/
- std::vector<std::string> GetArgs(const std::string& strArg) const;
+ std::vector<std::string> GetArgs(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return true if the given argument has been manually set
@@ -257,7 +257,7 @@ protected:
* @param strArg Argument to get (e.g. "-foo")
* @return true if the argument has been set
*/
- bool IsArgSet(const std::string& strArg) const;
+ bool IsArgSet(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return true if the argument was originally passed as a negated option,
@@ -266,7 +266,7 @@ protected:
* @param strArg Argument to get (e.g. "-foo")
* @return true if the argument was passed negated
*/
- bool IsArgNegated(const std::string& strArg) const;
+ bool IsArgNegated(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return string argument or default value
@@ -275,8 +275,8 @@ protected:
* @param strDefault (e.g. "1")
* @return command-line argument or default value
*/
- std::string GetArg(const std::string& strArg, const std::string& strDefault) const;
- std::optional<std::string> GetArg(const std::string& strArg) const;
+ std::string GetArg(const std::string& strArg, const std::string& strDefault) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
+ std::optional<std::string> GetArg(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return path argument or default value
@@ -288,7 +288,7 @@ protected:
* for examples or implementation for details). If argument is empty or not
* set, default_value is returned unchanged.
*/
- fs::path GetPathArg(std::string arg, const fs::path& default_value = {}) const;
+ fs::path GetPathArg(std::string arg, const fs::path& default_value = {}) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return integer argument or default value
@@ -298,13 +298,13 @@ protected:
* @return command-line argument (0 if invalid number) or default value
*/
template <std::integral Int>
- Int GetArg(const std::string& strArg, Int nDefault) const;
+ Int GetArg(const std::string& strArg, Int nDefault) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
template <std::integral Int>
- std::optional<Int> GetArg(const std::string& strArg) const;
+ std::optional<Int> GetArg(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
- int64_t GetIntArg(const std::string& strArg, int64_t nDefault) const { return GetArg<int64_t>(strArg, nDefault); }
- std::optional<int64_t> GetIntArg(const std::string& strArg) const { return GetArg<int64_t>(strArg); }
+ int64_t GetIntArg(const std::string& strArg, int64_t nDefault) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args) { return GetArg<int64_t>(strArg, nDefault); }
+ std::optional<int64_t> GetIntArg(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args) { return GetArg<int64_t>(strArg); }
/**
* Return boolean argument or default value
@@ -313,8 +313,8 @@ protected:
* @param fDefault (true or false)
* @return command-line argument or default value
*/
- bool GetBoolArg(const std::string& strArg, bool fDefault) const;
- std::optional<bool> GetBoolArg(const std::string& strArg) const;
+ bool GetBoolArg(const std::string& strArg, bool fDefault) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
+ std::optional<bool> GetBoolArg(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Set an argument if it doesn't already have a value
@@ -323,7 +323,7 @@ protected:
* @param strValue Value (e.g. "1")
* @return true if argument gets set, false if it already had a value
*/
- bool SoftSetArg(const std::string& strArg, const std::string& strValue);
+ bool SoftSetArg(const std::string& strArg, const std::string& strValue) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Set a boolean argument if it doesn't already have a value
@@ -332,97 +332,97 @@ protected:
* @param fValue Value (e.g. false)
* @return true if argument gets set, false if it already had a value
*/
- bool SoftSetBoolArg(const std::string& strArg, bool fValue);
+ bool SoftSetBoolArg(const std::string& strArg, bool fValue) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
// Forces an arg setting. Called by SoftSetArg() if the arg hasn't already
// been set. Also called directly in testing.
- void ForceSetArg(const std::string& strArg, const std::string& strValue);
+ void ForceSetArg(const std::string& strArg, const std::string& strValue) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Returns the appropriate chain type from the program arguments.
* @return ChainType::MAIN by default; raises runtime error if an invalid
* combination, or unknown chain is given.
*/
- ChainType GetChainType() const;
+ ChainType GetChainType() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Returns the appropriate chain type string from the program arguments.
* @return ChainType::MAIN string by default; raises runtime error if an
* invalid combination is given.
*/
- std::string GetChainTypeString() const;
+ std::string GetChainTypeString() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Add argument
*/
- void AddArg(const std::string& name, const std::string& help, unsigned int flags, const OptionsCategory& cat);
+ void AddArg(const std::string& name, const std::string& help, unsigned int flags, const OptionsCategory& cat) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Add subcommand
*/
- void AddCommand(const std::string& cmd, const std::string& help);
+ void AddCommand(const std::string& cmd, const std::string& help) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Add many hidden arguments
*/
- void AddHiddenArgs(const std::vector<std::string>& args);
+ void AddHiddenArgs(const std::vector<std::string>& args) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Clear available arguments
*/
- void ClearArgs();
+ void ClearArgs() EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Check CLI command args
*
* @throws std::runtime_error when multiple CLI_COMMAND arguments are specified
*/
- void CheckMultipleCLIArgs() const;
+ void CheckMultipleCLIArgs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get the help string
*/
- std::string GetHelpMessage() const;
+ std::string GetHelpMessage() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return Flags for known arg.
* Return default flags for unknown arg.
*/
- std::optional<unsigned int> GetArgFlags(const std::string& name) const;
+ std::optional<unsigned int> GetArgFlags(const std::string& name) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Set default flags to return for an unknown arg.
*/
- void SetDefaultFlags(std::optional<unsigned int>);
+ void SetDefaultFlags(std::optional<unsigned int>) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get settings file path, or return false if read-write settings were
* disabled with -nosettings.
*/
- bool GetSettingsPath(fs::path* filepath = nullptr, bool temp = false, bool backup = false) const;
+ bool GetSettingsPath(fs::path* filepath = nullptr, bool temp = false, bool backup = false) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Read settings file. Push errors to vector, or log them if null.
*/
- bool ReadSettingsFile(std::vector<std::string>* errors = nullptr);
+ bool ReadSettingsFile(std::vector<std::string>* errors = nullptr) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Write settings file or backup settings file. Push errors to vector, or
* log them if null.
*/
- bool WriteSettingsFile(std::vector<std::string>* errors = nullptr, bool backup = false) const;
+ bool WriteSettingsFile(std::vector<std::string>* errors = nullptr, bool backup = false) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get current setting from config file or read/write settings file,
* ignoring nonpersistent command line or forced settings values.
*/
- common::SettingsValue GetPersistentSetting(const std::string& name) const;
+ common::SettingsValue GetPersistentSetting(const std::string& name) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Access settings with lock held.
*/
template <typename Fn>
- void LockSettings(Fn&& fn)
+ void LockSettings(Fn&& fn) EXCLUSIVE_LOCKS_REQUIRED(!cs_args)
{
LOCK(cs_args);
fn(m_settings);
@@ -432,7 +432,7 @@ protected:
* Log the config file options and the command line arguments,
* useful for troubleshooting.
*/
- void LogArgs() const;
+ void LogArgs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
private:
// Internal helpers, for use by callers that already hold `cs_args`.
@@ -454,7 +454,7 @@ private:
* name was set. Raise an exception if an invalid combination of flags was
* provided.
*/
- std::variant<ChainType, std::string> GetChainArg() const;
+ std::variant<ChainType, std::string> GetChainArg() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
// Helper function for LogArgs().
void logArgsPrefix(
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.