sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-25 12:02:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#22263: refactor: wrap CCoinsViewCursor in unique_ptr

Browse Source 
7ad414f4bfa74595ee5726e66f3527045c02a977 doc: add comment about CCoinsViewDBCursor constructor (James O'Beirne)
0f8a5a4dd530549d37c43da52c923ac3b2af1a03 move-only(ish): don't expose CCoinsViewDBCursor (James O'Beirne)
615c1adfb07b9b466173166dc2e53ace540e4b32 refactor: wrap CCoinsViewCursor in unique_ptr (James O'Beirne)

Pull request description:

  I tripped over this one for a few hours at the beginning of the week, so I've sort of got a personal vendetta against `CCoinsView::Cursor()` returning a raw pointer.

  Specifically in the case of CCoinsViewDB, if a raw cursor is allocated and not freed, a cryptic leveldb assertion failure occurs on CCoinsViewDB destruction (`Assertion 'dummy_versions_.next_ == &dummy_versions_' failed.`).

  This is a pretty simple change.

  Related to: https://github.com/bitcoin/bitcoin/issues/21766
  See also: https://github.com/google/leveldb/issues/142#issuecomment-414418135

ACKs for top commit:
  MarcoFalke:
    review ACK 7ad414f4bfa74595ee5726e66f3527045c02a977 🔎
  jonatack:
    re-ACK 7ad414f4bfa74595ee5726e66f3527045c02a977 modulo suggestion
  ryanofsky:
    Code review ACK 7ad414f4bfa74595ee5726e66f3527045c02a977. Two new commits look good and thanks for clarifying constructor comment

Tree-SHA512: 6471d03e2de674d84b1ea0d31e25f433d52aa1aa4996f7b4aab1bd02b6bc340b15e64cc8ea07bbefefa3b5da35384ca5400cc230434e787c30931b8574c672f9
This commit is contained in:
MarcoFalke 2021-06-23 18:32:28 +02:00 committed by pasta
parent 56af7d6727
commit 52c8ef2feb
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
6 changed files with 37 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/coins.cpp
View File

@ -13,7 +13,7 @@ bool CCoinsView::GetCoin(const COutPoint &outpoint, Coin &coin) const { return f
uint256 CCoinsView::GetBestBlock() const { return uint256(); }
std::vector<uint256> CCoinsView::GetHeadBlocks() const { return std::vector<uint256>(); }
bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return false; }
CCoinsViewCursor *CCoinsView::Cursor() const { return nullptr; }
std::unique_ptr<CCoinsViewCursor> CCoinsView::Cursor() const { return nullptr; }
bool CCoinsView::HaveCoin(const COutPoint &outpoint) const
{
@ -28,7 +28,7 @@ uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); }
std::vector<uint256> CCoinsViewBacked::GetHeadBlocks() const { return base->GetHeadBlocks(); }
void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; }
bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return base->BatchWrite(mapCoins, hashBlock); }
CCoinsViewCursor *CCoinsViewBacked::Cursor() const { return base->Cursor(); }
std::unique_ptr<CCoinsViewCursor> CCoinsViewBacked::Cursor() const { return base->Cursor(); }
size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); }
CCoinsViewCache::CCoinsViewCache(CCoinsView *baseIn) : CCoinsViewBacked(baseIn), cachedCoinsUsage(0) {}

6
src/coins.h
View File

@ -180,7 +180,7 @@ public:
virtual bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock);
//! Get a cursor to iterate over the whole state
virtual CCoinsViewCursor *Cursor() const;
virtual std::unique_ptr<CCoinsViewCursor> Cursor() const;
//! As we use CCoinsViews polymorphically, have a virtual destructor
virtual ~CCoinsView() {}
@ -204,7 +204,7 @@ public:
std::vector<uint256> GetHeadBlocks() const override;
void SetBackend(CCoinsView &viewIn);
bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override;
CCoinsViewCursor *Cursor() const override;
std::unique_ptr<CCoinsViewCursor> Cursor() const override;
size_t EstimateSize() const override;
};
@ -237,7 +237,7 @@ public:
uint256 GetBestBlock() const override;
void SetBestBlock(const uint256 &hashBlock);
bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override;
CCoinsViewCursor* Cursor() const override {
std::unique_ptr<CCoinsViewCursor> Cursor() const override {
throw std::logic_error("CCoinsViewCache cursor iteration not supported.");
}

4
src/rpc/blockchain.cpp
View File

@ -2734,7 +2734,7 @@ UniValue scantxoutset(const JSONRPCRequest& request)
LOCK(cs_main);
CChainState& active_chainstate = chainman.ActiveChainstate();
active_chainstate.ForceFlushStateToDisk();
pcursor = std::unique_ptr<CCoinsViewCursor>(active_chainstate.CoinsDB().Cursor());
pcursor = active_chainstate.CoinsDB().Cursor();
CHECK_NONFATAL(pcursor);
tip = active_chainstate.m_chain.Tip();
CHECK_NONFATAL(tip);
@ -2923,7 +2923,7 @@ UniValue CreateUTXOSnapshot(NodeContext& node, CChainState& chainstate, CAutoFil
throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set");
}
pcursor = std::unique_ptr<CCoinsViewCursor>(chainstate.CoinsDB().Cursor());
pcursor = chainstate.CoinsDB().Cursor();
tip = chainstate.m_blockman.LookupBlockIndex(stats.hashBlock);
CHECK_NONFATAL(tip);
}

4
src/test/fuzz/coins_view.cpp
View File

@ -180,8 +180,8 @@ FUZZ_TARGET_INIT(coins_view, initialize_coins_view)
}
{
const CCoinsViewCursor* coins_view_cursor = backend_coins_view.Cursor();
assert(coins_view_cursor == nullptr);
std::unique_ptr<CCoinsViewCursor> coins_view_cursor = backend_coins_view.Cursor();
assert(!coins_view_cursor);
(void)backend_coins_view.EstimateSize();
(void)backend_coins_view.GetBestBlock();
(void)backend_coins_view.GetHeadBlocks();

29
src/txdb.cpp
View File

@ -172,9 +172,34 @@ bool CBlockTreeDB::ReadLastBlockFile(int &nFile) {
return Read(DB_LAST_BLOCK, nFile);
}
CCoinsViewCursor *CCoinsViewDB::Cursor() const
/** Specialization of CCoinsViewCursor to iterate over a CCoinsViewDB */
class CCoinsViewDBCursor: public CCoinsViewCursor
{
CCoinsViewDBCursor *i = new CCoinsViewDBCursor(const_cast<CDBWrapper&>(*m_db).NewIterator(), GetBestBlock());
public:
// Prefer using CCoinsViewDB::Cursor() since we want to perform some
// cache warmup on instantiation.
CCoinsViewDBCursor(CDBIterator* pcursorIn, const uint256&hashBlockIn):
CCoinsViewCursor(hashBlockIn), pcursor(pcursorIn) {}
~CCoinsViewDBCursor() {}
bool GetKey(COutPoint &key) const override;
bool GetValue(Coin &coin) const override;
unsigned int GetValueSize() const override;
bool Valid() const override;
void Next() override;
private:
std::unique_ptr<CDBIterator> pcursor;
std::pair<char, COutPoint> keyTmp;
friend class CCoinsViewDB;
};
std::unique_ptr<CCoinsViewCursor> CCoinsViewDB::Cursor() const
{
auto i = std::make_unique<CCoinsViewDBCursor>(
const_cast<CDBWrapper&>(*m_db).NewIterator(), GetBestBlock());
/* It seems that there are no "const iterators" for LevelDB. Since we
only need read operations on it, use a const-cast to get around
that restriction. */

24
src/txdb.h
View File

@ -63,7 +63,7 @@ public:
uint256 GetBestBlock() const override;
std::vector<uint256> GetHeadBlocks() const override;
bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override;
CCoinsViewCursor *Cursor() const override;
std::unique_ptr<CCoinsViewCursor> Cursor() const override;
//! Attempt to update from an older database format. Returns whether an error occurred.
bool Upgrade();
@ -73,28 +73,6 @@ public:
void ResizeCache(size_t new_cache_size) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
};
/** Specialization of CCoinsViewCursor to iterate over a CCoinsViewDB */
class CCoinsViewDBCursor: public CCoinsViewCursor
{
public:
~CCoinsViewDBCursor() {}
bool GetKey(COutPoint &key) const override;
bool GetValue(Coin &coin) const override;
unsigned int GetValueSize() const override;
bool Valid() const override;
void Next() override;
private:
CCoinsViewDBCursor(CDBIterator* pcursorIn, const uint256 &hashBlockIn):
CCoinsViewCursor(hashBlockIn), pcursor(pcursorIn) {}
std::unique_ptr<CDBIterator> pcursor;
std::pair<char, COutPoint> keyTmp;
friend class CCoinsViewDB;
};
/** Access to the block database (blocks/index/) */
class CBlockTreeDB : public CDBWrapper
{