From 569353068568444a25b301bbd6513bb510157dc9 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 24 Jul 2019 13:23:48 -0400 Subject: refactor: have CCoins* data managed under CChainState This change encapsulates UTXO set data within CChainState instances, removing global data `pcoinsTip` and `pcoinsviewdb`. This is necessary if we want to maintain multiple chainstates with their own rendering of the UTXO set. We introduce a class CoinsViews which consolidates the construction of a CCoins* hierarchy. Construction of its various pieces (db, coinscatcher, in-memory cache) is split up so that we avoid flushing bad state to disk if startup is interrupted. We also introduce `CChainState::CanFlushToDisk()` which tells us when it is safe to flush the chainstate based on this partial construction. This commit could be broken into smaller pieces, but it would require more ephemeral diffs to, e.g., temporarily change CCoinsViewDB's constructor invocations. Other changes: - A parameter has been added to the CCoinsViewDB constructor that allows the name of the corresponding leveldb directory to be specified. Thanks to Russell Yanofsky and Marco Falke for helpful feedback. --- src/init.cpp | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) (limited to 'src/init.cpp') diff --git a/src/init.cpp b/src/init.cpp index bb3ff8d88..50ae0c625 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -149,7 +148,6 @@ NODISCARD static bool CreatePidFile() // shutdown thing. // -static std::unique_ptr pcoinscatcher; static std::unique_ptr globalVerifyHandle; static boost::thread_group threadGroup; @@ -234,8 +232,11 @@ void Shutdown(InitInterfaces& interfaces) } // FlushStateToDisk generates a ChainStateFlushed callback, which we should avoid missing - if (pcoinsTip != nullptr) { - ::ChainstateActive().ForceFlushStateToDisk(); + // + // g_chainstate is referenced here directly (instead of ::ChainstateActive()) because it + // may not have been initialized yet. + if (g_chainstate && g_chainstate->CanFlushToDisk()) { + g_chainstate->ForceFlushStateToDisk(); } // After there are no more peers/RPC left to give us new data which may generate @@ -250,12 +251,10 @@ void Shutdown(InitInterfaces& interfaces) { LOCK(cs_main); - if (pcoinsTip != nullptr) { - ::ChainstateActive().ForceFlushStateToDisk(); + if (g_chainstate && g_chainstate->CanFlushToDisk()) { + g_chainstate->ForceFlushStateToDisk(); + g_chainstate->ResetCoinsViews(); } - pcoinsTip.reset(); - pcoinscatcher.reset(); - pcoinsdbview.reset(); pblocktree.reset(); } for (const auto& client : interfaces.chain_clients) { @@ -1466,10 +1465,10 @@ bool AppInitMain(InitInterfaces& interfaces) bool is_coinsview_empty; try { LOCK(cs_main); + // This statement makes ::ChainstateActive() usable. + g_chainstate = MakeUnique(); UnloadBlockIndex(); - pcoinsTip.reset(); - pcoinsdbview.reset(); - pcoinscatcher.reset(); + // new CBlockTreeDB tries to delete the existing file, which // fails if it's still open from the previous loop. Close it first: pblocktree.reset(); @@ -1520,9 +1519,12 @@ bool AppInitMain(InitInterfaces& interfaces) // At this point we're either in reindex or we've loaded a useful // block tree into BlockIndex()! - pcoinsdbview.reset(new CCoinsViewDB(nCoinDBCache, false, fReset || fReindexChainState)); - pcoinscatcher.reset(new CCoinsViewErrorCatcher(pcoinsdbview.get())); - pcoinscatcher->AddReadErrCallback([]() { + ::ChainstateActive().InitCoinsDB( + /* cache_size_bytes */ nCoinDBCache, + /* in_memory */ false, + /* should_wipe */ fReset || fReindexChainState); + + ::ChainstateActive().CoinsErrorCatcher().AddReadErrCallback([]() { uiInterface.ThreadSafeMessageBox( _("Error reading from database, shutting down.").translated, "", CClientUIInterface::MSG_ERROR); @@ -1530,23 +1532,25 @@ bool AppInitMain(InitInterfaces& interfaces) // If necessary, upgrade from older database format. // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!pcoinsdbview->Upgrade()) { + if (!::ChainstateActive().CoinsDB().Upgrade()) { strLoadError = _("Error upgrading chainstate database").translated; break; } // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate - if (!ReplayBlocks(chainparams, pcoinsdbview.get())) { + if (!ReplayBlocks(chainparams, &::ChainstateActive().CoinsDB())) { strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.").translated; break; } // The on-disk coinsdb is now in a good state, create the cache - pcoinsTip.reset(new CCoinsViewCache(pcoinscatcher.get())); + ::ChainstateActive().InitCoinsCache(); + assert(::ChainstateActive().CanFlushToDisk()); - is_coinsview_empty = fReset || fReindexChainState || pcoinsTip->GetBestBlock().IsNull(); + is_coinsview_empty = fReset || fReindexChainState || + ::ChainstateActive().CoinsTip().GetBestBlock().IsNull(); if (!is_coinsview_empty) { - // LoadChainTip sets ::ChainActive() based on pcoinsTip's best block + // LoadChainTip sets ::ChainActive() based on CoinsTip()'s best block if (!LoadChainTip(chainparams)) { strLoadError = _("Error initializing block database").translated; break; @@ -1588,7 +1592,7 @@ bool AppInitMain(InitInterfaces& interfaces) break; } - if (!CVerifyDB().VerifyDB(chainparams, pcoinsdbview.get(), gArgs.GetArg("-checklevel", DEFAULT_CHECKLEVEL), + if (!CVerifyDB().VerifyDB(chainparams, &::ChainstateActive().CoinsDB(), gArgs.GetArg("-checklevel", DEFAULT_CHECKLEVEL), gArgs.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS))) { strLoadError = _("Corrupted block database detected").translated; break; -- cgit v1.2.3 From 582d2cd74754d6b9a2394616a9c82a89d2d71976 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 24 Jul 2019 13:39:42 -0400 Subject: Cover UTXO set access with lock annotations i.e. any CoinsViews members. Adds a lock acquisition to `gettxoutsetinfo` RPC to comply with added annotations. Co-authored-by: Russell Yanofsky --- src/init.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src/init.cpp') diff --git a/src/init.cpp b/src/init.cpp index 50ae0c625..97be4637a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -235,8 +235,11 @@ void Shutdown(InitInterfaces& interfaces) // // g_chainstate is referenced here directly (instead of ::ChainstateActive()) because it // may not have been initialized yet. - if (g_chainstate && g_chainstate->CanFlushToDisk()) { - g_chainstate->ForceFlushStateToDisk(); + { + LOCK(cs_main); + if (g_chainstate && g_chainstate->CanFlushToDisk()) { + g_chainstate->ForceFlushStateToDisk(); + } } // After there are no more peers/RPC left to give us new data which may generate -- cgit v1.2.3