Resolve scoped flush lock shutdown race.

include/bitcoin/blockchain/pools/orphan_pool_manager.hpp

@@ -82,7 +82,7 @@ class BCB_API orphan_pool_manager
 
     // These are thread safe.
     std::atomic<bool> stopped_;
-    const bool flush_;
+    const bool flush_reorganizations_;
     orphan_pool& orphan_pool_;
     threadpool thread_pool_;
     validate_block validator_;

src/interface/block_chain.cpp

@@ -914,12 +914,14 @@ bool block_chain::stopped() const
 // This is a performance optimization that requires write_serial(..., false).
 bool block_chain::begin_writes()
 {
+    //vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
     return database_.flush_lock();
 }
 
 bool block_chain::end_writes()
 {
     return database_.flush_unlock();
+    //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 }
 
 // private

src/pools/orphan_pool_manager.cpp

@@ -52,7 +52,7 @@ orphan_pool_manager::orphan_pool_manager(threadpool& thread_pool,
     const settings& settings)
   : fast_chain_(chain),
     stopped_(true),
-    flush_(settings.flush_reorganizations),
+    flush_reorganizations_(settings.flush_reorganizations),
     orphan_pool_(orphan_pool),
     validator_(thread_pool, fast_chain_, settings),
     subscriber_(std::make_shared<reorganize_subscriber>(thread_pool, NAME)),
@@ -68,9 +68,11 @@ bool orphan_pool_manager::start()
     stopped_ = false;
     subscriber_->start();
 
-    ///////////////////////////////////////////////////////////////////////////
-    // Begin flush lock.
-    return flush_ || fast_chain_.begin_writes();
+    // Don't begin flush lock if flushing on each reorganization.
+    if (flush_reorganizations_)
+        return true;
+
+    return fast_chain_.begin_writes();
 }
 
 bool orphan_pool_manager::stop()
@@ -80,8 +82,15 @@ bool orphan_pool_manager::stop()
     subscriber_->stop();
     subscriber_->invoke(error::service_stopped, 0, {}, {});
 
-    return flush_ || fast_chain_.end_writes();
-    // End flush lock.
+    // Don't end flush lock if flushing on each reorganization.
+    if (flush_reorganizations_)
+        return true;
+
+    // Ensure no reorganization is in process when the flush lock is cleared.
+    ///////////////////////////////////////////////////////////////////////////
+    // Critical Section
+    shared_lock lock(mutex_);
+    return fast_chain_.end_writes();
     ///////////////////////////////////////////////////////////////////////////
 }
 
@@ -114,10 +123,8 @@ void orphan_pool_manager::organize(block_const_ptr block,
 
     ///////////////////////////////////////////////////////////////////////////
     // Critical Section.
-    //
-    //  Use scope lock to protect the fast chain from concurrent organizations.
-    // This has no impact on direct use of either blockchain interface.
-    //
+    // Use scope lock to guard the chain against concurrent organizations.
+    // If a reorganization started after stop it will stop before writing.
     const auto lock = std::make_shared<scope_lock>(mutex_);
 
     const result_handler locked_handler =
@@ -128,7 +135,7 @@ void orphan_pool_manager::organize(block_const_ptr block,
     if (fast_chain_.get_block_exists(block->hash()) ||
         !orphan_pool_.add(block))
     {
-        locked_handler(error::duplicate);
+        locked_handler(error::duplicate_block);
         return;
     }
 
@@ -138,7 +145,7 @@ void orphan_pool_manager::organize(block_const_ptr block,
     if (fork->empty())
     {
         // There is no link so the block is currently an orphan.
-        locked_handler(error::orphan);
+        locked_handler(error::orphan_block);
         return;
     }
 
@@ -150,7 +157,6 @@ void orphan_pool_manager::complete(const code& ec, scope_lock::ptr lock,
     result_handler handler)
 {
     lock.reset();
-    //
     // End Critical Section.
     ///////////////////////////////////////////////////////////////////////////
 
@@ -315,7 +321,7 @@ void orphan_pool_manager::organized(fork::ptr fork, result_handler handler)
     // Replace! Switch!
     //#########################################################################
     const auto swap = fast_chain_.swap(outgoing_blocks, fork->blocks(),
-        fork->height(), fork->hash(), flush_);
+        fork->height(), fork->hash(), flush_reorganizations_);
     //#########################################################################
 
     validate_block::report(fork->blocks().back(), start_time, "deposited");

src/pools/transaction_pool.cpp

@@ -142,6 +142,7 @@ void transaction_pool::do_validate(transaction_const_ptr tx,
 
     handler(error::operation_failed, {});
 
+    // TODO: restore
     ////validator_->validate(tx,
     ////    std::bind(&transaction_pool::handle_validated,
     ////        this, _1, _2, tx, validator, handler));
@@ -157,7 +158,7 @@ void transaction_pool::handle_validated(const code& ec,
         return;
     }
 
-    if (ec == error::input_not_found || ec == error::validate_inputs_failed)
+    if (ec == error::missing_input || ec == error::validate_inputs_failed)
     {
         BITCOIN_ASSERT(unconfirmed.size() == 1);
         handler(ec, unconfirmed);
@@ -203,7 +204,7 @@ void transaction_pool::do_organize(const code& ec, const indexes& unconfirmed,
     // Recheck for existence under lock, as a duplicate may have been added.
     if (is_in_pool(tx->hash()))
     {
-        handle_validate(error::duplicate, {});
+        handle_validate(error::duplicate_pool_transaction, {});
         return;
     }
 
@@ -381,7 +382,7 @@ void transaction_pool::add(transaction_const_ptr tx, result_handler handler)
 {
     // When a new tx is added to the buffer drop the oldest.
     if (maintain_consistency_ && buffer_.size() == buffer_.capacity())
-        delete_package(error::pool_filled);
+        delete_package(error::transaction_pool_filled);
 
     tx->validation.confirm = handler;
     buffer_.push_back(tx);

src/validation/fork.cpp

@@ -71,7 +71,7 @@ block_const_ptr_list fork::pop(size_t index, const code& reason)
         const auto block = *it;
         block->header().validation.height = header::validation::orphan_height;
         block->validation.result = it == start ? reason :
-            error::previous_block_invalid;
+            error::invalid_previous_block;
         out.push_back(block);
     }
 

src/validation/validate_block.cpp

@@ -207,7 +207,7 @@ void validate_block::connect_inputs(transaction::sets_const_ptr input_sets,
 
         if (!input.previous_output().validation.cache.is_valid())
         {
-            ec = error::input_not_found;
+            ec = error::missing_input;
             break;
         }
 

src/validation/validate_input.cpp

@@ -70,7 +70,7 @@ code validate_input::convert_result(verify_result_type result)
         case verify_result_type::verify_result_stack_size:
         case verify_result_type::verify_result_sig_count:
         case verify_result_type::verify_result_pubkey_count:
-            return error::size_limits;
+            return error::block_size_limit;
 
         // Failed verify operations.
         case verify_result_type::verify_result_verify:
@@ -88,7 +88,7 @@ code validate_input::convert_result(verify_result_type result)
         case verify_result_type::verify_result_unbalanced_conditional:
             return error::validate_inputs_failed;
 
-        // BIP62 errors (should not see these unless requsted).
+        // BIP62 errors (should not see these unless requested).
         case verify_result_type::verify_result_sig_hashtype:
         case verify_result_type::verify_result_sig_der:
         case verify_result_type::verify_result_minimaldata:

test/transaction_pool.cpp

@@ -377,8 +377,8 @@ BOOST_AUTO_TEST_CASE(transaction_pool__add__empty__one)
 
 BOOST_AUTO_TEST_CASE(transaction_pool__add__overflow_two__oldest_two_removed_expected_callbacks)
 {
-    DECLARE_TRANSACTION(0, error::pool_filled);
-    DECLARE_TRANSACTION(1, error::pool_filled);
+    DECLARE_TRANSACTION(0, error::transaction_pool_filled);
+    DECLARE_TRANSACTION(1, error::transaction_pool_filled);
     DECLARE_TRANSACTION(2, error::service_stopped);
     transaction_pool_fixture::buffer buffer(1);
     DECLARE_TRANSACTION_POOL(mempool, buffer);
@@ -395,9 +395,9 @@ BOOST_AUTO_TEST_CASE(transaction_pool__add__overflow_two__oldest_two_removed_exp
 
 BOOST_AUTO_TEST_CASE(transaction_pool__add__overflow_with_dependencies__removes_oldest_and_dependencies)
 {
-    DECLARE_TRANSACTION(0, error::pool_filled);
-    DECLARE_TRANSACTION(1, error::pool_filled);
-    DECLARE_TRANSACTION(2, error::pool_filled);
+    DECLARE_TRANSACTION(0, error::transaction_pool_filled);
+    DECLARE_TRANSACTION(1, error::transaction_pool_filled);
+    DECLARE_TRANSACTION(2, error::transaction_pool_filled);
     DECLARE_TRANSACTION(3, error::service_stopped);
     ADD_INPUT_TO_TX_NUMBER(1, hash0, 42);
     ADD_INPUT_TO_TX_NUMBER(2, hash1, 24);
@@ -409,9 +409,9 @@ BOOST_AUTO_TEST_CASE(transaction_pool__add__overflow_with_dependencies__removes_
     mempool.add(tx3, handle_confirm3);
     BOOST_REQUIRE_EQUAL(mempool.transactions().size(), 1u);
     BOOST_REQUIRE_EQUAL(TX_ID_AT_POSITION(mempool, 0), tx3_id);
-    REQUIRE_CALLBACK(0, error::pool_filled);
-    REQUIRE_CALLBACK(1, error::pool_filled);
-    REQUIRE_CALLBACK(2, error::pool_filled);
+    REQUIRE_CALLBACK(0, error::transaction_pool_filled);
+    REQUIRE_CALLBACK(1, error::transaction_pool_filled);
+    REQUIRE_CALLBACK(2, error::transaction_pool_filled);
 }
 
 BOOST_AUTO_TEST_SUITE_END()