Skip to content

Conversation

@practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Nov 22, 2017

Add missing cs_wallet locks when accessing m_last_block_processed.

m_last_block_processed is guarded by cs_wallet.

These changes are required to get the Travis CI build (the build job with Clang's thread safety analysis enabled) to pass when the following locking annotation (see #11226) is added:

const CBlockIndex* m_last_block_processed GUARDED_BY(cs_wallet);

@maflcko
Copy link
Member

maflcko commented Nov 22, 2017

Since this helps review, might as well add the GUARDED_BY annotation to prevent further patches that touch the same line.

@practicalswift practicalswift force-pushed the missing-m_last_block_processed-locks branch from 77f1239 to e9d0a71 Compare November 22, 2017 20:58
@practicalswift
Copy link
Contributor Author

@MarcoFalke Good idea! Fixed and pushed :-)

m_last_block_processed is guarded by cs_wallet.

These changes are required to get the Travis CI build (the build
job with Clang's thread safety analysis enabled) to pass when the
following locking annotation is added:

```
const CBlockIndex* m_last_block_processed GUARDED_BY(cs_wallet);
```
@practicalswift practicalswift force-pushed the missing-m_last_block_processed-locks branch from e9d0a71 to e737100 Compare November 22, 2017 20:59
@practicalswift practicalswift changed the title Add missing cs_wallet locks when accessing m_last_block_processed wallet: Add missing cs_wallet locks when accessing m_last_block_processed Nov 22, 2017
@promag
Copy link
Contributor

promag commented Nov 22, 2017

utACK e737100. Looks good!

@promag
Copy link
Contributor

promag commented Feb 3, 2018

Sorry @practicalswift, it's a NACK. At the moment m_last_block_processed is guarded by cs_main. See

bitcoin/src/wallet/wallet.cpp

Lines 1280 to 1282 in 85123be

// We could also take cs_wallet here, and call m_last_block_processed
// protected by cs_wallet instead of cs_main, but as long as we need
// cs_main here anyway, its easier to just call it cs_main-protected.

@practicalswift practicalswift deleted the missing-m_last_block_processed-locks branch April 10, 2021 19:33
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants