This commit is contained in:
Matias Furszyfer 2024-04-29 04:33:05 +02:00 committed by GitHub
commit e856af4261
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 189 additions and 38 deletions

View File

@ -145,6 +145,7 @@ BITCOIN_TESTS =\
test/serfloat_tests.cpp \
test/serialize_tests.cpp \
test/settings_tests.cpp \
test/sharedlock_tests.cpp \
test/sighash_tests.cpp \
test/sigopcount_tests.cpp \
test/skiplist_tests.cpp \

View File

@ -55,6 +55,9 @@ static void BlockAssemblerAddPackageTxns(benchmark::Bench& bench)
node::BlockAssembler::Options assembler_options;
assembler_options.test_block_validity = false;
// Connect genesis block
assert(testing_setup->m_node.chainman->ActiveChainstate().LoadGenesisBlock());
bench.run([&] {
PrepareBlock(testing_setup->m_node, P2WSH_OP_TRUE, assembler_options);
});

View File

@ -7,6 +7,8 @@
#include <tinyformat.h>
#include <util/time.h>
SharedMutex g_cs_blockindex_data;
std::string CBlockFileInfo::ToString() const
{
return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, FormatISO8601Date(nTimeFirst), FormatISO8601Date(nTimeLast));
@ -18,6 +20,14 @@ std::string CBlockIndex::ToString() const
pprev, nHeight, hashMerkleRoot.ToString(), GetBlockHash().ToString());
}
void CBlockIndex::SetFileData(int file_num, int data_pos, int undo_pos)
{
LOCK(g_cs_blockindex_data);
nFile = file_num;
nDataPos = data_pos;
nUndoPos = undo_pos;
}
void CChain::SetTip(CBlockIndex& block)
{
CBlockIndex* pindex = &block;

View File

@ -19,6 +19,7 @@
#include <algorithm>
#include <cassert>
#include <cstdint>
#include <shared_mutex>
#include <string>
#include <vector>
@ -132,6 +133,8 @@ enum BlockStatus : uint32_t {
//!< ancestors before they were validated, and unset when they were validated.
};
extern SharedMutex g_cs_blockindex_data;
/** The block chain is a tree shaped structure starting with the
* genesis block at the root, with each block potentially having multiple
* candidates to be the next block. A blockindex may have multiple pprev pointing
@ -153,13 +156,13 @@ public:
int nHeight{0};
//! Which # file this block is stored in (blk?????.dat)
int nFile GUARDED_BY(::cs_main){0};
int nFile GUARDED_BY(g_cs_blockindex_data){-1};
//! Byte offset within blk?????.dat where this block's data is stored
unsigned int nDataPos GUARDED_BY(::cs_main){0};
unsigned int nDataPos GUARDED_BY(g_cs_blockindex_data){0};
//! Byte offset within rev?????.dat where this block's undo data is stored
unsigned int nUndoPos GUARDED_BY(::cs_main){0};
unsigned int nUndoPos GUARDED_BY(g_cs_blockindex_data){0};
//! (memory only) Total amount of work (expected number of hashes) in the chain up to and including this block
arith_uint256 nChainWork{};
@ -206,26 +209,42 @@ public:
{
}
void SetFileData(int file_num, int data_pos, int undo_pos);
void SetUndoPos(int undo_pos) {
LOCK(g_cs_blockindex_data);
nUndoPos = undo_pos;
}
int GetFileNum() const {
LOCK_SHARED(g_cs_blockindex_data);
return nFile;
}
int GetDataPos() const {
LOCK_SHARED(g_cs_blockindex_data);
return nDataPos;
}
FlatFilePos GetFilePos(bool is_undo) const
{
LOCK_SHARED(g_cs_blockindex_data);
FlatFilePos ret;
if (nFile >= 0) {
ret.nFile = nFile;
ret.nPos = is_undo ? nUndoPos : nDataPos;
}
return ret;
}
FlatFilePos GetBlockPos() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
FlatFilePos ret;
if (nStatus & BLOCK_HAVE_DATA) {
ret.nFile = nFile;
ret.nPos = nDataPos;
}
return ret;
return nStatus & BLOCK_HAVE_DATA ? GetFilePos(/*is_undo=*/false) : FlatFilePos{};
}
FlatFilePos GetUndoPos() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
AssertLockHeld(::cs_main);
FlatFilePos ret;
if (nStatus & BLOCK_HAVE_UNDO) {
ret.nFile = nFile;
ret.nPos = nUndoPos;
}
return ret;
return nStatus & BLOCK_HAVE_UNDO ? GetFilePos(/*is_undo=*/true) : FlatFilePos{};
}
CBlockHeader GetBlockHeader() const
@ -384,6 +403,7 @@ public:
READWRITE(VARINT_MODE(obj.nHeight, VarIntMode::NONNEGATIVE_SIGNED));
READWRITE(VARINT(obj.nStatus));
READWRITE(VARINT(obj.nTx));
LOCK_SHARED(g_cs_blockindex_data);
if (obj.nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO)) READWRITE(VARINT_MODE(obj.nFile, VarIntMode::NONNEGATIVE_SIGNED));
if (obj.nStatus & BLOCK_HAVE_DATA) READWRITE(VARINT(obj.nDataPos));
if (obj.nStatus & BLOCK_HAVE_UNDO) READWRITE(VARINT(obj.nUndoPos));

View File

@ -18,9 +18,8 @@ interfaces::BlockInfo MakeBlockInfo(const CBlockIndex* index, const CBlock* data
info.prev_hash = index->pprev ? index->pprev->phashBlock : nullptr;
info.height = index->nHeight;
info.chain_time_max = index->GetBlockTimeMax();
LOCK(::cs_main);
info.file_number = index->nFile;
info.data_pos = index->nDataPos;
info.file_number = index->GetFileNum();
info.data_pos = index->GetDataPos();
}
info.data = data;
return info;

View File

@ -117,6 +117,7 @@ bool BlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, s
if (pcursor->GetValue(diskindex)) {
// Construct block index object
CBlockIndex* pindexNew = insertBlockIndex(diskindex.ConstructBlockHash());
LOCK(g_cs_blockindex_data);
pindexNew->pprev = insertBlockIndex(diskindex.hashPrev);
pindexNew->nHeight = diskindex.nHeight;
pindexNew->nFile = diskindex.nFile;
@ -242,12 +243,10 @@ void BlockManager::PruneOneBlockFile(const int fileNumber)
for (auto& entry : m_block_index) {
CBlockIndex* pindex = &entry.second;
if (pindex->nFile == fileNumber) {
if (pindex->GetFileNum() == fileNumber) {
pindex->nStatus &= ~BLOCK_HAVE_DATA;
pindex->nStatus &= ~BLOCK_HAVE_UNDO;
pindex->nFile = 0;
pindex->nDataPos = 0;
pindex->nUndoPos = 0;
pindex->SetFileData(/*file_num=*/-1, /*data_pos=*/0, /*undo_pos=*/0);
m_dirty_blockindex.insert(pindex);
// Prune from m_blocks_unlinked -- any block we prune would have
@ -522,9 +521,15 @@ bool BlockManager::LoadBlockIndexDB(const std::optional<uint256>& snapshot_block
// Check presence of blk files
LogPrintf("Checking all blk files are present...\n");
std::set<int> setBlkDataFiles;
for (const auto& [_, block_index] : m_block_index) {
for (auto& [_, block_index] : m_block_index) {
if (block_index.nStatus & BLOCK_HAVE_DATA) {
setBlkDataFiles.insert(block_index.nFile);
setBlkDataFiles.insert(block_index.GetFileNum());
} else {
// In case we don't have the block, the position must be -1.
// (applies to older clients that set 'nFile=0' during pruning)
if (block_index.GetFileNum() == 0) {
block_index.SetFileData(/*file_num=*/-1, /*data_pos=*/0, /*undo_pos=*/0);
}
}
}
for (std::set<int>::iterator it = setBlkDataFiles.begin(); it != setBlkDataFiles.end(); it++) {
@ -702,8 +707,10 @@ bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos
bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& index) const
{
const FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())};
if (index.nHeight == 0) return false; // nothing to do
LOCK_SHARED(g_cs_blockindex_data); // keep lock until we finish reading data from disk
const FlatFilePos pos = index.GetFilePos(/*is_undo=*/true);
if (pos.IsNull()) {
LogError("%s: no undo data available\n", __func__);
return false;
@ -1003,7 +1010,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid
// Write undo information to disk
if (block.GetUndoPos().IsNull()) {
FlatFilePos _pos;
if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo) + 40)) {
if (!FindUndoPos(state, block.GetFileNum(), _pos, ::GetSerializeSize(blockundo) + 40)) {
LogError("ConnectBlock(): FindUndoPos failed\n");
return false;
}
@ -1028,7 +1035,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid
cursor.undo_height = block.nHeight;
}
// update nUndoPos in block index
block.nUndoPos = _pos.nPos;
block.SetUndoPos(_pos.nPos);
block.nStatus |= BLOCK_HAVE_UNDO;
m_dirty_blockindex.insert(&block);
}
@ -1072,11 +1079,16 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons
bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) const
{
const FlatFilePos block_pos{WITH_LOCK(cs_main, return index.GetBlockPos())};
FlatFilePos block_pos;
if (!ReadBlockFromDisk(block, block_pos)) {
return false;
{
LOCK_SHARED(g_cs_blockindex_data); // keep lock until we finish reading the block from disk
block_pos = index.GetFilePos(/*is_undo=*/false);
if (!ReadBlockFromDisk(block, block_pos)) {
return false;
}
}
if (block.GetHash() != index.GetBlockHash()) {
LogError("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s\n",
index.ToString(), block_pos.ToString());

View File

@ -586,6 +586,11 @@ static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex& blockin
}
}
// No need to go to disk to get the genesis block
if (blockindex.nHeight == 0) {
return Params().GenesisBlock();
}
if (!blockman.ReadBlockFromDisk(block, blockindex)) {
// Block not found on disk. This could be because we have the block
// header in our index but not yet have the block or did not accept the

View File

@ -208,8 +208,10 @@ void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexTyp
}
template void EnterCritical(const char*, const char*, int, Mutex*, bool);
template void EnterCritical(const char*, const char*, int, RecursiveMutex*, bool);
template void EnterCritical(const char*, const char*, int, SharedMutex*, bool);
template void EnterCritical(const char*, const char*, int, std::mutex*, bool);
template void EnterCritical(const char*, const char*, int, std::recursive_mutex*, bool);
template void EnterCritical(const char*, const char*, int, std::shared_mutex*, bool);
void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line)
{

View File

@ -16,6 +16,7 @@
#include <condition_variable>
#include <mutex>
#include <shared_mutex>
#include <string>
#include <thread>
@ -111,6 +112,7 @@ public:
}
using unique_lock = std::unique_lock<PARENT>;
using shared_lock = std::shared_lock<PARENT>;
#ifdef __clang__
//! For negative capabilities in the Clang Thread Safety Analysis.
//! A negative requirement uses the EXCLUSIVE_LOCKS_REQUIRED attribute, in conjunction
@ -139,6 +141,8 @@ using Mutex = AnnotatedMixin<std::mutex>;
*/
class GlobalMutex : public Mutex { };
using SharedMutex = AnnotatedMixin<std::shared_mutex>;
#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs)
inline void AssertLockNotHeldInline(const char* name, const char* file, int line, Mutex* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) { AssertLockNotHeldInternal(name, file, line, cs); }
@ -242,6 +246,43 @@ public:
#define REVERSE_LOCK(g) typename std::decay<decltype(g)>::type::reverse_lock UNIQUE_NAME(revlock)(g, #g, __FILE__, __LINE__)
/** Wrapper around std::shared_lock style lock for MutexType.
* (for now, it does not keep track of the concurrent threads that acquire the shared lock) */
template <typename MutexType>
class SCOPED_LOCKABLE SharedLock : public MutexType::shared_lock
{
private:
using Base = typename MutexType::shared_lock;
void Enter(const char* pszName, const char* pszFile, int nLine)
{
#ifdef DEBUG_LOCKCONTENTION
if (Base::try_lock()) return;
LOG_TIME_MICROS_WITH_CATEGORY(strprintf("shared lock contention %s, %s:%d", pszName, pszFile, nLine), BCLog::LOCK);
#endif
Base::lock();
}
bool TryEnter(const char* pszName, const char* pszFile, int nLine) { return Base::try_lock(); }
public:
SharedLock(MutexType& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) SHARED_LOCK_FUNCTION(mutexIn) : Base(mutexIn, std::defer_lock)
{
if (fTry)
TryEnter(pszName, pszFile, nLine);
else
Enter(pszName, pszFile, nLine);
}
~SharedLock() UNLOCK_FUNCTION()
{
if (Base::owns_lock())
Base::unlock();
}
operator bool() { return Base::owns_lock(); }
};
// When locking a Mutex, require negative capability to ensure the lock
// is not already held
inline Mutex& MaybeCheckNotHeld(Mutex& cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; }
@ -261,6 +302,8 @@ inline MutexType* MaybeCheckNotHeld(MutexType* m) LOCKS_EXCLUDED(m) LOCK_RETURNE
#define TRY_LOCK(cs, name) UniqueLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true)
#define WAIT_LOCK(cs, name) UniqueLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
#define LOCK_SHARED(cs) SharedLock UNIQUE_NAME(shared_block1)(cs, #cs, __FILE__, __LINE__)
#define ENTER_CRITICAL_SECTION(cs) \
{ \
EnterCritical(#cs, __FILE__, __LINE__, &cs); \

View File

@ -0,0 +1,57 @@
// Copyright (c) 2023 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or https://www.opensource.org/licenses/mit-license.php.
#include <sync.h>
#include <test/util/setup_common.h>
#include <boost/test/unit_test.hpp>
#include <thread>
BOOST_AUTO_TEST_SUITE(sharedlock_tests)
void run_task_in_parallel(const std::function<void()>& task, int num_threads)
{
std::vector<std::thread> workers;
for (int i=0; i<num_threads; i++) {
workers.emplace_back(std::thread(task));
}
std::for_each(workers.begin(), workers.end(), [](std::thread &t) { t.join(); });
}
BOOST_AUTO_TEST_CASE(sharedlock_basics)
{
SharedMutex mutex;
{
// 1) Acquire shared lock and check that the writer lock cannot be acquired
LOCK_SHARED(mutex);
BOOST_CHECK(!mutex.try_lock());
}
{
// 2) Acquire exclusive lock and check that the reader lock cannot be acquired
LOCK(mutex);
BOOST_CHECK(!mutex.try_lock_shared());
}
{
// 3) Acquire exclusive lock and verify that no other thread can acquire it.
LOCK(mutex);
run_task_in_parallel([&](){
assert(!mutex.try_lock());
assert(!mutex.try_lock_shared());
}, /*num_threads=*/10);
}
{
// 4) Acquire shared lock and verify that multiple reader threads can acquire the shared mutex at the same time.
LOCK_SHARED(mutex);
run_task_in_parallel([&](){
LOCK_SHARED(mutex);
assert(!mutex.try_lock());
}, /*num_threads=*/10);
}
}
BOOST_AUTO_TEST_SUITE_END()

View File

@ -8,17 +8,18 @@
#include <node/blockstorage.h>
#include <primitives/block.h>
#include <undo.h>
#include <validation.h>
using node::BlockManager;
bool ComputeFilter(BlockFilterType filter_type, const CBlockIndex& block_index, BlockFilter& filter, const BlockManager& blockman)
{
LOCK(::cs_main);
CBlock block;
if (!blockman.ReadBlockFromDisk(block, block_index.GetBlockPos())) {
return false;
{
LOCK_SHARED(g_cs_blockindex_data); // keep lock until we finish reading the block from disk
if (!blockman.ReadBlockFromDisk(block, block_index.GetFilePos(/*is_undo=*/false))) {
return false;
}
}
CBlockUndo block_undo;

View File

@ -3701,9 +3701,7 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd
pindexNew->nHeight, pindexNew->nChainTx, prev_tx_sum(*pindexNew), PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT);
pindexNew->nChainTx = 0;
}
pindexNew->nFile = pos.nFile;
pindexNew->nDataPos = pos.nPos;
pindexNew->nUndoPos = 0;
pindexNew->SetFileData(/*file_num=*/pos.nFile, /*data_pos=*/pos.nPos, /*undo_pos=*/0);
pindexNew->nStatus |= BLOCK_HAVE_DATA;
if (DeploymentActiveAt(*pindexNew, *this, Consensus::DEPLOYMENT_SEGWIT)) {
pindexNew->nStatus |= BLOCK_OPT_WITNESS;