Remove redundant LOCK(…) and AssertLockHeld(…) #11361

pull practicalswift wants to merge 2 commits into bitcoin:master from practicalswift:remove-redundant-locks changing 2 files +0 −2

practicalswift commented at 9:23 PM on September 18, 2017: contributor
Remove redundant LOCK(…) and AssertLockHeld(…):
- Remove LOCK(cs_args) from ArgsManager::SoftSetArg(...)
- Remove AssertLockHeld(cs_wallet) from CWallet::RescanFromTime(...)

Remove AssertLockHeld(cs_wallet) from CWallet::RescanFromTime(...)

cs_wallet not needed. ScanForWalletTransactions(...) takes it itself.

See https://github.com/bitcoin/bitcoin/pull/11226#issuecomment-329863375

5f4e12df29

Remove LOCK(cs_args) from ArgsManager::SoftSetArg(...)

cs_args not needed. IsArgSet(...) and ForceSetArg(...) take it themselves.

ba14f93edc

jonasschnelli added the label Refactoring on Sep 19, 2017

in src/wallet/wallet.cpp:1533 in ba14f93edc

1529 | @@ -1530,7 +1530,6 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
1530 |  int64_t CWallet::RescanFromTime(int64_t startTime, bool update)
1531 |  {
1532 |      AssertLockHeld(cs_main);
1533 | -    AssertLockHeld(cs_wallet);

promag commented at 2:19 PM on September 19, 2017:

This is changed in #11281.

practicalswift commented at 2:38 PM on September 19, 2017:

Yes, #11281 resolves this. Thanks for pointing that out!

promag commented at 2:25 PM on September 19, 2017: member

NACK, see comments.
in src/util.cpp:462 in ba14f93edc
```
 458 | @@ -459,7 +459,6 @@ bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const
 459 |  
 460 |  bool ArgsManager::SoftSetArg(const std::string& strArg, const std::string& strValue)
 461 |  {
 462 | -    LOCK(cs_args);
```
promag commented at 2:25 PM on September 19, 2017:

Without this, theoretically there is race to ForceSetArg below:

two threads get false from IsArgSet("foo");

one will override the other's value.
practicalswift commented at 2:37 PM on September 19, 2017:

Ah, good point. Thanks for reviewing!
promag commented at 2:25 PM on September 19, 2017: member

Ops, missed this comment.
practicalswift commented at 2:38 PM on September 19, 2017: contributor

Closing since no longer needed. Thanks for @promag for reviewing :-)
practicalswift closed this on Sep 19, 2017
practicalswift deleted the branch on Apr 10, 2021
DrahtBot locked this on Aug 18, 2022

Contributors

practicalswift

promag

Labels

Refactoring