From eab6110ab69ac86c4d51a22eee076f23d3876a02 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 8 Feb 2019 10:35:06 -0500 Subject: [PATCH] Merge #15365: wallet: Add lock annotation for mapAddressBook faa46475d7 wallet: Add lock annotation for mapAddressBook (MarcoFalke) Pull request description: This adds lock annotations for `mapAddressBook` and also moves one lock from inside `GetDestValues` to the caller to be in line with the other methods (`eraseDestData`, `addDestData`, ...) Tree-SHA512: cef9397523e2f5717d4a9a6b2da1fe07042484a51b3c067ae64425768637f334350a2c3db4ab7e00af99b2a587f6b656b68ee1195f6a3db6d47298d0b2b6174a --- src/interfaces/wallet.cpp | 1 + src/qt/test/addressbooktests.cpp | 1 + src/wallet/rpcwallet.cpp | 2 +- src/wallet/wallet.cpp | 5 ++--- src/wallet/wallet.h | 14 +++++++------- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 2cdab7c37c..b3501afcc3 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -264,6 +264,7 @@ public: } std::vector getDestValues(const std::string& prefix) override { + LOCK(m_wallet.cs_wallet); return m_wallet.GetDestValues(prefix); } void lockCoin(const COutPoint& output) override diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index b12d58a1f2..56a4b5ab45 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -92,6 +92,7 @@ void TestAddAddressesToSendBook() } auto check_addbook_size = [wallet](int expected_size) { + LOCK(wallet->cs_wallet); QCOMPARE(static_cast(wallet->mapAddressBook.size()), expected_size); }; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 0d23aa9e36..0344606ffb 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1726,7 +1726,7 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) * @param ret The UniValue into which the result is stored. * @param filter The "is mine" filter bool. */ -static void ListTransactions(CWallet * const pwallet, const CWalletTx& wtx, const std::string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, const std::string& strAccount, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pwallet->cs_wallet) { CAmount nFee; std::string strSentAccount; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a4ac6784e9..91bdf84b46 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4165,7 +4165,7 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s { bool fUpdated = false; { - LOCK(cs_wallet); // mapAddressBook + LOCK(cs_wallet); std::map::iterator mi = mapAddressBook.find(address); fUpdated = mi != mapAddressBook.end(); mapAddressBook[address].name = strName; @@ -4182,7 +4182,7 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s bool CWallet::DelAddressBook(const CTxDestination& address) { { - LOCK(cs_wallet); // mapAddressBook + LOCK(cs_wallet); // Delete destdata tuples associated with address std::string strAddress = EncodeDestination(address); @@ -4916,7 +4916,6 @@ bool CWallet::GetDestData(const CTxDestination &dest, const std::string &key, st std::vector CWallet::GetDestValues(const std::string& prefix) const { - LOCK(cs_wallet); std::vector values; for (const auto& address : mapAddressBook) { for (const auto& data : address.second.destdata) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ba9bb6e21e..2f3fda8748 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -905,7 +905,7 @@ public: int64_t nOrderPosNext GUARDED_BY(cs_wallet) = 0; uint64_t nAccountingEntryNumber = 0; - std::map mapAddressBook; + std::map mapAddressBook GUARDED_BY(cs_wallet); std::set setLockedCoins GUARDED_BY(cs_wallet); @@ -1013,15 +1013,15 @@ public: bool LoadCScript(const CScript& redeemScript); //! Adds a destination data tuple to the store, and saves it to disk - bool AddDestData(const CTxDestination &dest, const std::string &key, const std::string &value); + bool AddDestData(const CTxDestination& dest, const std::string& key, const std::string& value) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Erases a destination data tuple in the store and on disk - bool EraseDestData(const CTxDestination &dest, const std::string &key); + bool EraseDestData(const CTxDestination& dest, const std::string& key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Adds a destination data tuple to the store, without saving it to disk - void LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value); + void LoadDestData(const CTxDestination& dest, const std::string& key, const std::string& value) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Look up a destination data tuple in the store, return true if found false otherwise - bool GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const; + bool GetDestData(const CTxDestination& dest, const std::string& key, std::string* value) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Get all destination values matching a prefix. - std::vector GetDestValues(const std::string& prefix) const; + std::vector GetDestValues(const std::string& prefix) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Adds a watch-only address to the store, and saves it to disk. bool AddWatchOnly(const CScript& dest, int64_t nCreateTime) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -1185,7 +1185,7 @@ public: bool DelAddressBook(const CTxDestination& address); - const std::string& GetLabelName(const CScript& scriptPubKey) const; + const std::string& GetLabelName(const CScript& scriptPubKey) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void GetScriptForMining(std::shared_ptr &script);