mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-01-10 20:03:34 -03:00
protect g_chainman with cs_main
I'd previously attempted to create a specialized lock for ChainstateManager, but it turns out that because that lock would be required for functions like ChainActive() and ChainstateActive(), it created irreconcilable lock inversions since those functions are used so broadly throughout the codebase. Instead, I'm just using cs_main to protect the contents of g_chainman. Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
This commit is contained in:
parent
2b081c4568
commit
c9017ce3bc
5 changed files with 48 additions and 11 deletions
21
src/init.cpp
21
src/init.cpp
|
@ -709,11 +709,17 @@ static void ThreadImport(std::vector<fs::path> vImportFiles)
|
|||
}
|
||||
|
||||
// scan for better chains in the block chain database, that are not yet connected in the active best chain
|
||||
BlockValidationState state;
|
||||
if (!ActivateBestChain(state, chainparams)) {
|
||||
LogPrintf("Failed to connect best block (%s)\n", state.ToString());
|
||||
StartShutdown();
|
||||
return;
|
||||
|
||||
// We can't hold cs_main during ActivateBestChain even though we're accessing
|
||||
// the g_chainman unique_ptrs since ABC requires us not to be holding cs_main, so retrieve
|
||||
// the relevant pointers before the ABC call.
|
||||
for (CChainState* chainstate : WITH_LOCK(::cs_main, return g_chainman.GetAll())) {
|
||||
BlockValidationState state;
|
||||
if (!chainstate->ActivateBestChain(state, chainparams, nullptr)) {
|
||||
LogPrintf("Failed to connect best block (%s)\n", state.ToString());
|
||||
StartShutdown();
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (gArgs.GetBoolArg("-stopafterblockimport", DEFAULT_STOPAFTERBLOCKIMPORT)) {
|
||||
|
@ -1622,8 +1628,9 @@ bool AppInitMain(NodeContext& node)
|
|||
}
|
||||
|
||||
bool failed_rewind{false};
|
||||
|
||||
for (CChainState* chainstate : g_chainman.GetAll()) {
|
||||
// Can't hold cs_main while calling RewindBlockIndex, so retrieve the relevant
|
||||
// chainstates beforehand.
|
||||
for (CChainState* chainstate : WITH_LOCK(::cs_main, return g_chainman.GetAll())) {
|
||||
if (!fReset) {
|
||||
// Note that RewindBlockIndex MUST run even if we're about to -reindex-chainstate.
|
||||
// It both disconnects blocks based on the chainstate, and drops block data in
|
||||
|
|
|
@ -82,7 +82,7 @@ void AppTests::appTests()
|
|||
// Reset global state to avoid interfering with later tests.
|
||||
AbortShutdown();
|
||||
UnloadBlockIndex();
|
||||
g_chainman.Reset();
|
||||
WITH_LOCK(::cs_main, g_chainman.Reset());
|
||||
}
|
||||
|
||||
//! Entry point for BitcoinGUI tests.
|
||||
|
|
|
@ -81,12 +81,14 @@ ChainstateManager g_chainman;
|
|||
|
||||
CChainState& ChainstateActive()
|
||||
{
|
||||
LOCK(::cs_main);
|
||||
assert(g_chainman.m_active_chainstate);
|
||||
return *g_chainman.m_active_chainstate;
|
||||
}
|
||||
|
||||
CChain& ChainActive()
|
||||
{
|
||||
LOCK(::cs_main);
|
||||
return ::ChainstateActive().m_chain;
|
||||
}
|
||||
|
||||
|
@ -1295,6 +1297,7 @@ static CBlockIndex *pindexBestForkTip = nullptr, *pindexBestForkBase = nullptr;
|
|||
|
||||
BlockMap& BlockIndex()
|
||||
{
|
||||
LOCK(::cs_main);
|
||||
return g_chainman.m_blockman.m_block_index;
|
||||
}
|
||||
|
||||
|
@ -4704,7 +4707,7 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi
|
|||
// Activate the genesis block so normal node progress can continue
|
||||
if (hash == chainparams.GetConsensus().hashGenesisBlock) {
|
||||
BlockValidationState state;
|
||||
if (!ActivateBestChain(state, chainparams)) {
|
||||
if (!ActivateBestChain(state, chainparams, nullptr)) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -823,14 +823,41 @@ private:
|
|||
//! free the chainstate contents immediately after it finishes validation
|
||||
//! to cautiously avoid a case where some other part of the system is still
|
||||
//! using this pointer (e.g. net_processing).
|
||||
//!
|
||||
//! Once this pointer is set to a corresponding chainstate, it will not
|
||||
//! be reset until init.cpp:Shutdown(). This means it is safe to acquire
|
||||
//! the contents of this pointer with ::cs_main held, release the lock,
|
||||
//! and then use the reference without concern of it being deconstructed.
|
||||
//!
|
||||
//! This is especially important when, e.g., calling ActivateBestChain()
|
||||
//! on all chainstates because we are not able to hold ::cs_main going into
|
||||
//! that call.
|
||||
std::unique_ptr<CChainState> m_ibd_chainstate;
|
||||
|
||||
//! A chainstate initialized on the basis of a UTXO snapshot. If this is
|
||||
//! non-null, it is always our active chainstate.
|
||||
//!
|
||||
//! Once this pointer is set to a corresponding chainstate, it will not
|
||||
//! be reset until init.cpp:Shutdown(). This means it is safe to acquire
|
||||
//! the contents of this pointer with ::cs_main held, release the lock,
|
||||
//! and then use the reference without concern of it being deconstructed.
|
||||
//!
|
||||
//! This is especially important when, e.g., calling ActivateBestChain()
|
||||
//! on all chainstates because we are not able to hold ::cs_main going into
|
||||
//! that call.
|
||||
std::unique_ptr<CChainState> m_snapshot_chainstate;
|
||||
|
||||
//! Points to either the ibd or snapshot chainstate; indicates our
|
||||
//! most-work chain.
|
||||
//!
|
||||
//! Once this pointer is set to a corresponding chainstate, it will not
|
||||
//! be reset until init.cpp:Shutdown(). This means it is safe to acquire
|
||||
//! the contents of this pointer with ::cs_main held, release the lock,
|
||||
//! and then use the reference without concern of it being deconstructed.
|
||||
//!
|
||||
//! This is especially important when, e.g., calling ActivateBestChain()
|
||||
//! on all chainstates because we are not able to hold ::cs_main going into
|
||||
//! that call.
|
||||
CChainState* m_active_chainstate{nullptr};
|
||||
|
||||
//! If true, the assumed-valid chainstate has been fully validated
|
||||
|
@ -894,7 +921,7 @@ public:
|
|||
void Reset();
|
||||
};
|
||||
|
||||
extern ChainstateManager g_chainman;
|
||||
extern ChainstateManager g_chainman GUARDED_BY(::cs_main);
|
||||
|
||||
/** @returns the most-work valid chainstate. */
|
||||
CChainState& ChainstateActive();
|
||||
|
|
|
@ -454,7 +454,7 @@ public:
|
|||
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
|
||||
wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock());
|
||||
{
|
||||
LOCK(wallet->cs_wallet);
|
||||
LOCK2(::cs_main, wallet->cs_wallet);
|
||||
wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
|
||||
}
|
||||
bool firstRun;
|
||||
|
|
Loading…
Reference in a new issue