laantungir
/
bitcoin
mirror of https://github.com/bitcoin/bitcoin.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

validation: make CCheckQueueControl's CCheckQueue non-optional 

This simplifies the construction logic and will allow the constructor and
destructor to lock and unlock uncondiationally.
This commit is contained in:
Cory Fields 2025-05-06 15:14:50 +00:00
parent 4c8c90b556
commit c3b0e6c7f4
6 changed files with 18 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/bench/checkqueue.cpp
View File

@ -56,7 +56,7 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench)
bench.minEpochIterations(10).batch(BATCH_SIZE * BATCHES).unit("job").run([&] {
// Make insecure_rand here so that each iteration is identical.
CCheckQueueControl<PrevectorJob> control(&queue);
CCheckQueueControl<PrevectorJob> control(queue);
for (auto vChecks : vBatches) {
control.Add(std::move(vChecks));
}

20
src/checkqueue.h
View File

@ -208,43 +208,35 @@ template <typename T, typename R = std::remove_cvref_t<decltype(std::declval<T>(
class CCheckQueueControl
{
private:
CCheckQueue<T, R> * const pqueue;
CCheckQueue<T, R>& m_queue;
bool fDone;
public:
CCheckQueueControl() = delete;
CCheckQueueControl(const CCheckQueueControl&) = delete;
CCheckQueueControl& operator=(const CCheckQueueControl&) = delete;
explicit CCheckQueueControl(CCheckQueue<T> * const pqueueIn) : pqueue(pqueueIn), fDone(false)
explicit CCheckQueueControl(CCheckQueue<T>& queueIn) : m_queue(queueIn), fDone(false)
{
// passed queue is supposed to be unused, or nullptr
if (pqueue != nullptr) {
ENTER_CRITICAL_SECTION(pqueue->m_control_mutex);
}
ENTER_CRITICAL_SECTION(m_queue.m_control_mutex);
}
std::optional<R> Complete()
{
if (pqueue == nullptr) return std::nullopt;
auto ret = pqueue->Complete();
auto ret = m_queue.Complete();
fDone = true;
return ret;
}
void Add(std::vector<T>&& vChecks)
{
if (pqueue != nullptr) {
pqueue->Add(std::move(vChecks));
}
m_queue.Add(std::move(vChecks));
}
~CCheckQueueControl()
{
if (!fDone)
Complete();
if (pqueue != nullptr) {
LEAVE_CRITICAL_SECTION(pqueue->m_control_mutex);
}
LEAVE_CRITICAL_SECTION(m_queue.m_control_mutex);
}
};

16
src/test/checkqueue_tests.cpp
View File

@ -165,7 +165,7 @@ void CheckQueueTest::Correct_Queue_range(std::vector<size_t> range)
for (const size_t i : range) {
size_t total = i;
FakeCheckCheckCompletion::n_calls = 0;
CCheckQueueControl<FakeCheckCheckCompletion> control(small_queue.get());
CCheckQueueControl<FakeCheckCheckCompletion> control(*small_queue);
while (total) {
vChecks.clear();
vChecks.resize(std::min<size_t>(total, m_rng.randrange(10)));
@ -220,7 +220,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure)
{
auto fixed_queue = std::make_unique<Fixed_Queue>(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS);
for (size_t i = 0; i < 1001; ++i) {
CCheckQueueControl<FixedCheck> control(fixed_queue.get());
CCheckQueueControl<FixedCheck> control(*fixed_queue);
size_t remaining = i;
while (remaining) {
size_t r = m_rng.randrange(10);
@ -246,7 +246,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure)
auto fail_queue = std::make_unique<Fixed_Queue>(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS);
for (auto times = 0; times < 10; ++times) {
for (const bool end_fails : {true, false}) {
CCheckQueueControl<FixedCheck> control(fail_queue.get());
CCheckQueueControl<FixedCheck> control(*fail_queue);
{
std::vector<FixedCheck> vChecks;
vChecks.resize(100, FixedCheck(std::nullopt));
@ -268,7 +268,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck)
size_t COUNT = 100000;
size_t total = COUNT;
{
CCheckQueueControl<UniqueCheck> control(queue.get());
CCheckQueueControl<UniqueCheck> control(*queue);
while (total) {
size_t r = m_rng.randrange(10);
std::vector<UniqueCheck> vChecks;
@ -300,7 +300,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory)
for (size_t i = 0; i < 1000; ++i) {
size_t total = i;
{
CCheckQueueControl<MemoryCheck> control(queue.get());
CCheckQueueControl<MemoryCheck> control(*queue);
while (total) {
size_t r = m_rng.randrange(10);
std::vector<MemoryCheck> vChecks;
@ -324,7 +324,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup)
auto queue = std::make_unique<FrozenCleanup_Queue>(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS);
bool fails = false;
std::thread t0([&]() {
CCheckQueueControl<FrozenCleanupCheck> control(queue.get());
CCheckQueueControl<FrozenCleanupCheck> control(*queue);
std::vector<FrozenCleanupCheck> vChecks(1);
control.Add(std::move(vChecks));
auto result = control.Complete(); // Hangs here
@ -364,7 +364,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
for (size_t i = 0; i < 3; ++i) {
tg.emplace_back(
[&]{
CCheckQueueControl<FakeCheck> control(queue.get());
CCheckQueueControl<FakeCheck> control(*queue);
// While sleeping, no other thread should execute to this point
auto observed = ++nThreads;
UninterruptibleSleep(std::chrono::milliseconds{10});
@ -387,7 +387,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
{
std::unique_lock<std::mutex> l(m);
tg.emplace_back([&]{
CCheckQueueControl<FakeCheck> control(queue.get());
CCheckQueueControl<FakeCheck> control(*queue);
std::unique_lock<std::mutex> ll(m);
has_lock = true;
cv.notify_one();

2
src/test/fuzz/checkqueue.cpp
View File

@ -49,7 +49,7 @@ FUZZ_TARGET(checkqueue)
(void)check_queue_1.Complete();
}
CCheckQueueControl<DumbCheck> check_queue_control{&check_queue_2};
CCheckQueueControl<DumbCheck> check_queue_control{check_queue_2};
if (fuzzed_data_provider.ConsumeBool()) {
check_queue_control.Add(std::move(checks_2));
}

2
src/test/transaction_tests.cpp
View File

@ -568,7 +568,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction)
// check all inputs concurrently, with the cache
PrecomputedTransactionData txdata(tx);
CCheckQueue<CScriptCheck> scriptcheckqueue(/*batch_size=*/128, /*worker_threads_num=*/20);
CCheckQueueControl<CScriptCheck> control(&scriptcheckqueue);
CCheckQueueControl<CScriptCheck> control(scriptcheckqueue);
std::vector<Coin> coins;
for(uint32_t i = 0; i < mtx.vin.size(); i++) {

2
src/validation.cpp
View File

@ -2612,7 +2612,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
// doesn't invalidate pointers into the vector, and keep txsdata in scope
// for as long as `control`.
std::optional<CCheckQueueControl<CScriptCheck>> control;
if (fScriptChecks && parallel_script_checks) control.emplace(&m_chainman.GetCheckQueue());
if (fScriptChecks && parallel_script_checks) control.emplace(m_chainman.GetCheckQueue());
std::vector<PrecomputedTransactionData> txsdata(block.vtx.size());