validation: make CCheckQueueControl's CCheckQueue non-optional

This simplifies the construction logic and will allow the constructor and
destructor to lock and unlock uncondiationally.

Author: theuni

src/bench/checkqueue.cpp

@@ -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));
         }

src/checkqueue.h

@@ -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);
     }
 };
 

src/test/checkqueue_tests.cpp

@@ -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();

src/test/fuzz/checkqueue.cpp

@@ -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));
     }

src/test/transaction_tests.cpp

@@ -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++) {

src/validation.cpp

@@ -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());