diff options
| author | Wladimir J. van der Laan <laanwj@gmail.com> | 2018-12-01 10:25:53 +0100 |
|---|---|---|
| committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2018-12-01 10:27:49 +0100 |
| commit | 5ab5341d13569736acc262ee750010d5c59edca5 (patch) | |
| tree | 26bfeb2f815fecc8dc76cce8f97f9893bf99feff | |
| parent | 011c42c5bd175e14b03741398f5d4f8a4cf3dee5 (diff) | |
| parent | c5ed6e73d3e18f26558c49fa3ecb1b7b0bb40d12 (diff) | |
| download | bitcoin-5ab5341d13569736acc262ee750010d5c59edca5.tar.xz | |
Merge #14841: consensus: Move CheckBlock() call to critical section
c5ed6e73d Move CheckBlock() call to critical section (Hennadii Stepanov)
Pull request description:
This is an alternative to #14803.
Refs:
- #14058
- #14072
- https://github.com/bitcoin/bitcoin/pull/14803#issuecomment-442233211 by @gmaxwell
> It doesn't support multithreaded validation and there are lot of things that prevent that, which is why I was concerned. Why doesn't the lock on the block index or even cs main prevent concurrency here?
- https://github.com/bitcoin/bitcoin/pull/14803#issuecomment-442237566 by @MarcoFalke
Tree-SHA512: 2152e97106e11da5763b2748234ecd2982daadab13a0da04215f4db60af802a44ab5700f32249137d122eb13fc2a02e0f2d561d364607d727d8c6ab879339afb
| -rw-r--r-- | src/validation.cpp | 8 | ||||
| -rw-r--r-- | test/sanitizer_suppressions/tsan | 3 |
2 files changed, 5 insertions, 6 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index 6333dd98d2..512a3619ca 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3530,12 +3530,14 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<cons CBlockIndex *pindex = nullptr; if (fNewBlock) *fNewBlock = false; CValidationState state; - // Ensure that CheckBlock() passes before calling AcceptBlock, as - // belt-and-suspenders. - bool ret = CheckBlock(*pblock, state, chainparams.GetConsensus()); + // CheckBlock() does not support multi-threaded block validation because CBlock::fChecked can cause data race. + // Therefore, the following critical section must include the CheckBlock() call as well. LOCK(cs_main); + // Ensure that CheckBlock() passes before calling AcceptBlock, as + // belt-and-suspenders. + bool ret = CheckBlock(*pblock, state, chainparams.GetConsensus()); if (ret) { // Store to disk ret = g_chainstate.AcceptBlock(pblock, state, chainparams, &pindex, fForceProcessing, nullptr, fNewBlock); diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan index 209c46f096..996f342eb9 100644 --- a/test/sanitizer_suppressions/tsan +++ b/test/sanitizer_suppressions/tsan @@ -1,9 +1,6 @@ # ThreadSanitizer suppressions # ============================ -# fChecked is theoretically racy, practically only in unit tests -race:CheckBlock - # WalletBatch (unidentified deadlock) deadlock:WalletBatch |