Fixes #11106
Also makes fReindex atomic as suggested in @TheBlueMatt comment below, and makes fUseCrypto atomic as suggested in 10916
https://github.com/bitcoin/bitcoin/pull/11107/commits/d291e7635b0ef4156c2805c6c4ee1adad91f0307 just renames the parameters in the txdb header file to make them consistent with those used in the cpp file, noticed it when looking for uses of fReindex
1631 | @@ -1632,8 +1632,12 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) 1632 | // ********************************************************* Step 11: start node 1633 | 1634 | //// debug print 1635 | - LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size()); 1636 | - LogPrintf("nBestHeight = %d\n", chainActive.Height()); 1637 | + {
I guess it would make sense to also cover mapBlockIndex and chainActive to make this move-safe.
chainActive.Height() is also accessed on line 1652, you want to bring it in too?
If chainActive is added then update descriptions accordingly.
Did you search for other cases?
No haven't modified the pull yet, it still just fixes the issue linked
Indeed, ight as well pull chainActive.Height() into the cs_main here.
Travis error looks unrelated, timeout in sendheaders.py - assert(wait_until(test_function, timeout=60))
While you're at it, want to make fReindex atomic so that the ThreadImport write doesn't "race" with net_processing reads?
Ping @TheBlueMatt if you have a bit of time :)
1637 | + { 1638 | + LOCK(cs_main); 1639 | + LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size()); 1640 | + } 1641 | + LogPrintf("nBestHeight = %d\n", chainActive.Height()); 1642 | +
nit: EOL whitespace here.
3522 | @@ -3523,7 +3523,7 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) 3523 | // Check whether we need to continue reindexing 3524 | bool fReindexing = false; 3525 | pblocktree->ReadReindexing(fReindexing); 3526 | - fReindex |= fReindexing; 3527 | + fReindex = fReindex | fReindexing;
Might as well use the atomic or here since we're making it atomic, no?
|=? It is only defined on atomic integral types I believe, not bool
Err, indeed, quivalent is to do an if(fReindexing) fReindex = true
Yeah true that's cleaner, will do
utACK 731065b114452ff770320d09639448b3c9a74b0a
3522 | @@ -3523,7 +3523,7 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) 3523 | // Check whether we need to continue reindexing 3524 | bool fReindexing = false; 3525 | pblocktree->ReadReindexing(fReindexing); 3526 | - fReindex |= fReindexing;
What's wrong with |= ?
See my reply to @TheBlueMatt's comment above, it's not implemented for atomic bool :)
utACK 731065b
Added a new commit to make fUseCrypto atomic as well, see #10916 (comment) @TheBlueMatt @jonasschnelli
Friendly ping @jonasschnelli and/or @TheBlueMatt if you have time, the last commit is only a 2 line change so shouldn't be too time consuming to review ;)
utACK c626dcb50eed496462fd4ac3e05bf79164749ebe
utACK https://github.com/bitcoin/bitcoin/commit/c626dcb50eed496462fd4ac3e05bf79164749ebe Generally "unrelated" cleanups (like 731065b114452ff770320d09639448b3c9a74b0a) should be avoided.