From 73e87feea781b05e465aa5702784532affa92208 Mon Sep 17 00:00:00 2001 From: 0xFFFC0000 <0xFFFC0000@proton.me> Date: Wed, 14 Feb 2024 21:09:10 +0000 Subject: [PATCH] Add ReadWrite locking mechanism on top of Blockchain. This will fix existing locking issues, and provide a foundation to implement more fine-grained locking mechanisms in future works. --- contrib/epee/include/syncobj.h | 5 + src/cryptonote_core/blockchain.cpp | 405 ++++++++++++++++++++++------- src/cryptonote_core/blockchain.h | 90 ++++++- 3 files changed, 400 insertions(+), 100 deletions(-) diff --git a/contrib/epee/include/syncobj.h b/contrib/epee/include/syncobj.h index 804bafda78e..23c98455b36 100644 --- a/contrib/epee/include/syncobj.h +++ b/contrib/epee/include/syncobj.h @@ -36,6 +36,7 @@ #include #include #include +#include namespace epee { @@ -157,6 +158,10 @@ namespace epee #define CRITICAL_REGION_END() } +typedef boost::shared_mutex ReadWriteMutex; +typedef boost::shared_lock ReadScope; +typedef boost::unique_lock ReadWriteScope; + } #endif diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 7c9bd916378..11fcaf5e500 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -99,7 +99,8 @@ Blockchain::Blockchain(tx_memory_pool& tx_pool) : m_btc_valid(false), m_batch_success(true), m_prepare_height(0), - m_rct_ver_cache() + m_rct_ver_cache(), + m_blockchain_transaction() { LOG_PRINT_L3("Blockchain::" << __func__); } @@ -113,21 +114,20 @@ Blockchain::~Blockchain() bool Blockchain::have_tx(const crypto::hash &id) const { LOG_PRINT_L3("Blockchain::" << __func__); - // WARNING: this function does not take m_blockchain_lock, and thus should only call read only - // m_db functions which do not depend on one another (ie, no getheight + gethash(height-1), as - // well as not accessing class members, even read only (ie, m_invalid_blocks). The caller must - // lock if it is otherwise needed. - return m_db->tx_exists(id); + m_blockchain_transaction.start_read(); + bool have_tx = m_db->tx_exists(id); + m_blockchain_transaction.end_read(); + return have_tx; + } //------------------------------------------------------------------ bool Blockchain::have_tx_keyimg_as_spent(const crypto::key_image &key_im) const { LOG_PRINT_L3("Blockchain::" << __func__); - // WARNING: this function does not take m_blockchain_lock, and thus should only call read only - // m_db functions which do not depend on one another (ie, no getheight + gethash(height-1), as - // well as not accessing class members, even read only (ie, m_invalid_blocks). The caller must - // lock if it is otherwise needed. - return m_db->has_key_image(key_im); + m_blockchain_transaction.start_write(); + bool have_tx_keyimg_as_spent = m_db->has_key_image(key_im); + m_blockchain_transaction.end_write(); + return have_tx_keyimg_as_spent; } //------------------------------------------------------------------ // This function makes sure that each "input" in an input (mixins) exists @@ -139,7 +139,7 @@ bool Blockchain::scan_outputkeys_for_indexes(size_t tx_version, const txin_to_ke LOG_PRINT_L3("Blockchain::" << __func__); // ND: Disable locking and make method private. - //CRITICAL_REGION_LOCAL(m_blockchain_lock); + // m_blockchain_transaction.start_write(); // verify that the input has key offsets (that it exists properly, really) if(!tx_in_to_key.key_offsets.size()) @@ -268,11 +268,10 @@ bool Blockchain::scan_outputkeys_for_indexes(size_t tx_version, const txin_to_ke uint64_t Blockchain::get_current_blockchain_height() const { LOG_PRINT_L3("Blockchain::" << __func__); - // WARNING: this function does not take m_blockchain_lock, and thus should only call read only - // m_db functions which do not depend on one another (ie, no getheight + gethash(height-1), as - // well as not accessing class members, even read only (ie, m_invalid_blocks). The caller must - // lock if it is otherwise needed. - return m_db->height(); + m_blockchain_transaction.start_write(); + uint64_t height = m_db->height(); + m_blockchain_transaction.end_write(); + return height; } //------------------------------------------------------------------ //FIXME: possibly move this into the constructor, to avoid accidentally @@ -284,7 +283,12 @@ bool Blockchain::init(BlockchainDB* db, const network_type nettype, bool offline CHECK_AND_ASSERT_MES(nettype != FAKECHAIN || test_options, false, "fake chain network type used without options"); CRITICAL_REGION_LOCAL(m_tx_pool); - CRITICAL_REGION_LOCAL1(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); + if (db == nullptr) { @@ -552,7 +556,11 @@ void Blockchain::pop_blocks(uint64_t nblocks) { uint64_t i = 0; CRITICAL_REGION_LOCAL(m_tx_pool); - CRITICAL_REGION_LOCAL1(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); bool stop_batch = m_db->batch_start(); @@ -593,7 +601,11 @@ void Blockchain::pop_blocks(uint64_t nblocks) block Blockchain::pop_block_from_blockchain() { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); m_timestamps_and_difficulties_height = 0; m_reset_timestamps_and_difficulties_height = true; @@ -679,7 +691,11 @@ block Blockchain::pop_block_from_blockchain() bool Blockchain::reset_and_set_genesis_block(const block& b) { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); m_timestamps_and_difficulties_height = 0; m_reset_timestamps_and_difficulties_height = true; invalidate_block_template_cache(); @@ -698,18 +714,19 @@ bool Blockchain::reset_and_set_genesis_block(const block& b) crypto::hash Blockchain::get_tail_id(uint64_t& height) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); - return m_db->top_block_hash(&height); + m_blockchain_transaction.start_write(); + crypto::hash top_block_hash = m_db->top_block_hash(&height); + m_blockchain_transaction.end_write(); + return top_block_hash; } //------------------------------------------------------------------ crypto::hash Blockchain::get_tail_id() const { LOG_PRINT_L3("Blockchain::" << __func__); - // WARNING: this function does not take m_blockchain_lock, and thus should only call read only - // m_db functions which do not depend on one another (ie, no getheight + gethash(height-1), as - // well as not accessing class members, even read only (ie, m_invalid_blocks). The caller must - // lock if it is otherwise needed. - return m_db->top_block_hash(); + m_blockchain_transaction.start_write(); + crypto::hash top_block_hash = m_db->top_block_hash(); + m_blockchain_transaction.end_write(); + return top_block_hash; } //------------------------------------------------------------------ /*TODO: this function was...poorly written. As such, I'm not entirely @@ -727,7 +744,11 @@ crypto::hash Blockchain::get_tail_id() const bool Blockchain::get_short_chain_history(std::list& ids) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); uint64_t i = 0; uint64_t current_multiplier = 1; uint64_t sz = m_db->height(); @@ -769,10 +790,11 @@ bool Blockchain::get_short_chain_history(std::list& ids) const crypto::hash Blockchain::get_block_id_by_height(uint64_t height) const { LOG_PRINT_L3("Blockchain::" << __func__); - // WARNING: this function does not take m_blockchain_lock, and thus should only call read only - // m_db functions which do not depend on one another (ie, no getheight + gethash(height-1), as - // well as not accessing class members, even read only (ie, m_invalid_blocks). The caller must - // lock if it is otherwise needed. + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); try { return m_db->get_block_hash_from_height(height); @@ -803,7 +825,11 @@ crypto::hash Blockchain::get_pending_block_id_by_height(uint64_t height) const bool Blockchain::get_block_by_hash(const crypto::hash &h, block &blk, bool *orphan) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); // try to find block in main chain try @@ -880,7 +906,11 @@ difficulty_type Blockchain::get_difficulty_for_next_block() } } - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); std::vector timestamps; std::vector difficulties; uint64_t height; @@ -1016,7 +1046,11 @@ size_t Blockchain::recalculate_difficulties(boost::optional start_heig return 0; } LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); const uint64_t start_height = start_height_opt ? *start_height_opt : check_difficulty_checkpoints().second; const uint64_t top_height = m_db->height() - 1; @@ -1102,7 +1136,11 @@ size_t Blockchain::recalculate_difficulties(boost::optional start_heig //------------------------------------------------------------------ std::vector Blockchain::get_last_block_timestamps(unsigned int blocks) const { - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); uint64_t height = m_db->height(); if (blocks > height) blocks = height; @@ -1118,7 +1156,11 @@ std::vector Blockchain::get_last_block_timestamps(unsigned int blocks) c bool Blockchain::rollback_blockchain_switching(std::list& original_chain, uint64_t rollback_height) { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); // fail if rollback_height passed is too high if (rollback_height > m_db->height()) @@ -1162,7 +1204,11 @@ bool Blockchain::rollback_blockchain_switching(std::list& original_chain, bool Blockchain::switch_to_alternative_blockchain(std::list& alt_chain, bool discard_disconnected_chain) { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); m_timestamps_and_difficulties_height = 0; m_reset_timestamps_and_difficulties_height = true; @@ -1303,7 +1349,11 @@ difficulty_type Blockchain::get_next_difficulty_for_alternative_chain(const std: // based on its blocks alone, need to get more blocks from the main chain if(alt_chain.size()< DIFFICULTY_BLOCKS_COUNT) { - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); // Figure out start and stop offsets for main chain blocks size_t main_chain_stop_offset = alt_chain.size() ? alt_chain.front().height : bei.height; @@ -1461,7 +1511,11 @@ bool Blockchain::validate_miner_transaction(const block& b, size_t cumulative_bl void Blockchain::get_last_n_blocks_weights(std::vector& weights, size_t count) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); auto h = m_db->height(); // this function is meaningless for an empty blockchain...granted it should never be empty @@ -1476,7 +1530,11 @@ void Blockchain::get_last_n_blocks_weights(std::vector& weights, size_ uint64_t Blockchain::get_long_term_block_weight_median(uint64_t start_height, size_t count) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); PERF_TIMER(get_long_term_block_weights); @@ -1555,7 +1613,11 @@ bool Blockchain::create_block_template(block& b, const crypto::hash *from_block, m_tx_pool.lock(); const auto unlock_guard = epee::misc_utils::create_scope_leave_handler([&]() { m_tx_pool.unlock(); }); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); if (m_btc_valid && !from_block) { // The pool cookie is atomic. The lack of locking is OK, as if it changes // just as we compare it, we'll just use a slightly old template, but @@ -1851,7 +1913,11 @@ bool Blockchain::complete_timestamps_vector(uint64_t start_top_height, std::vect if(timestamps.size() >= BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW) return true; - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); size_t need_elements = BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW - timestamps.size(); CHECK_AND_ASSERT_MES(start_top_height < m_db->height(), false, "internal error: passed start_height not < " << " m_db->height() -- " << start_top_height << " >= " << m_db->height()); size_t stop_offset = start_top_height > need_elements ? start_top_height - need_elements : 0; @@ -1928,7 +1994,11 @@ bool Blockchain::build_alt_chain(const crypto::hash &prev_id, std::list>& blocks, std::vector& txs) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); if(start_offset >= m_db->height()) return false; @@ -2174,7 +2248,11 @@ bool Blockchain::get_blocks(uint64_t start_offset, size_t count, std::vector>& blocks) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); const uint64_t height = m_db->height(); if(start_offset >= height) return false; @@ -2202,7 +2280,11 @@ bool Blockchain::get_blocks(uint64_t start_offset, size_t count, std::vector> blocks; @@ -2251,7 +2333,11 @@ bool Blockchain::handle_get_objects(NOTIFY_REQUEST_GET_OBJECTS::request& arg, NO bool Blockchain::get_alternative_blocks(std::vector& blocks) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); blocks.reserve(m_db->get_alt_block_count()); m_db->for_all_alt_blocks([&blocks](const crypto::hash &blkid, const cryptonote::alt_block_data_t &data, const cryptonote::blobdata_ref *blob) { @@ -2273,7 +2359,11 @@ bool Blockchain::get_alternative_blocks(std::vector& blocks) const size_t Blockchain::get_alternative_blocks_count() const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); return m_db->get_alt_block_count(); } //------------------------------------------------------------------ @@ -2307,7 +2397,11 @@ crypto::public_key Blockchain::get_output_key(uint64_t amount, uint64_t global_i bool Blockchain::get_outs(const COMMAND_RPC_GET_OUTPUTS_BIN::request& req, COMMAND_RPC_GET_OUTPUTS_BIN::response& res) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); res.outs.clear(); res.outs.reserve(req.outputs.size()); @@ -2416,7 +2510,11 @@ bool Blockchain::get_output_distribution(uint64_t amount, uint64_t from_height, bool Blockchain::find_blockchain_supplement(const std::list& qblock_ids, uint64_t& starter_offset) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); // make sure the request includes at least the genesis block, otherwise // how can we expect to sync from the client that the block list came from? @@ -2470,10 +2568,11 @@ bool Blockchain::find_blockchain_supplement(const std::list& qbloc difficulty_type Blockchain::block_difficulty(uint64_t i) const { LOG_PRINT_L3("Blockchain::" << __func__); - // WARNING: this function does not take m_blockchain_lock, and thus should only call read only - // m_db functions which do not depend on one another (ie, no getheight + gethash(height-1), as - // well as not accessing class members, even read only (ie, m_invalid_blocks). The caller must - // lock if it is otherwise needed. + m_blockchain_transaction.start_read(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_read(); + }); try { return m_db->get_block_difficulty(i); @@ -2494,7 +2593,11 @@ template& txs_ids, std::vector& txs, std::vector& missed_txs, bool pruned) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); txs.reserve(txs_ids.size()); for (const auto& tx_hash : txs_ids) @@ -2603,7 +2710,11 @@ bool Blockchain::get_transactions_blobs(const std::vector& txs_ids bool Blockchain::get_transactions_blobs(const std::vector& txs_ids, std::vector& txs, std::vector& missed_txs, bool pruned) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); txs.reserve(txs_ids.size()); for (const auto& tx_hash : txs_ids) @@ -2639,7 +2750,11 @@ template bool Blockchain::get_split_transactions_blobs(const t_ids_container& txs_ids, t_tx_container& txs, t_missed_container& missed_txs) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); reserve_container(txs, txs_ids.size()); for (const auto& tx_hash : txs_ids) @@ -2673,7 +2788,11 @@ template bool Blockchain::get_transactions(const t_ids_container& txs_ids, t_tx_container& txs, t_missed_container& missed_txs, bool pruned) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); reserve_container(txs, txs_ids.size()); for (const auto& tx_hash : txs_ids) @@ -2709,7 +2828,11 @@ bool Blockchain::get_transactions(const t_ids_container& txs_ids, t_tx_container bool Blockchain::find_blockchain_supplement(const std::list& qblock_ids, std::vector& hashes, std::vector* weights, uint64_t& start_height, uint64_t& current_height, bool clip_pruned) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); // if we can't find the split point, return false if(!find_blockchain_supplement(qblock_ids, start_height)) @@ -2748,7 +2871,11 @@ bool Blockchain::find_blockchain_supplement(const std::list& qbloc bool Blockchain::find_blockchain_supplement(const std::list& qblock_ids, bool clip_pruned, NOTIFY_RESPONSE_CHAIN_ENTRY::request& resp) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); bool result = find_blockchain_supplement(qblock_ids, resp.m_block_ids, &resp.m_block_weights, resp.start_height, resp.total_height, clip_pruned); if (result) @@ -2768,7 +2895,11 @@ bool Blockchain::find_blockchain_supplement(const std::list& qbloc bool Blockchain::find_blockchain_supplement(const uint64_t req_start_block, const std::list& qblock_ids, std::vector, std::vector > > >& blocks, uint64_t& total_height, uint64_t& start_height, bool pruned, bool get_miner_tx_hash, size_t max_block_count, size_t max_tx_count) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); // if a specific start height has been requested if(req_start_block > 0) @@ -2808,7 +2939,11 @@ bool Blockchain::add_block_as_invalid(const block& bl, const crypto::hash& h) bool Blockchain::add_block_as_invalid(const block_extended_info& bei, const crypto::hash& h) { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); auto i_res = m_invalid_blocks.insert(std::map::value_type(h, bei)); CHECK_AND_ASSERT_MES(i_res.second, false, "at insertion invalid by tx returned status existed"); MINFO("BLOCK ADDED AS INVALID: " << h << std::endl << ", prev_id=" << bei.bl.prev_id << ", m_invalid_blocks count=" << m_invalid_blocks.size()); @@ -2818,16 +2953,21 @@ bool Blockchain::add_block_as_invalid(const block_extended_info& bei, const cryp void Blockchain::flush_invalid_blocks() { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); m_invalid_blocks.clear(); } //------------------------------------------------------------------ bool Blockchain::have_block_unlocked(const crypto::hash& id, int *where) const { - // WARNING: this function does not take m_blockchain_lock, and thus should only call read only - // m_db functions which do not depend on one another (ie, no getheight + gethash(height-1), as - // well as not accessing class members, even read only (ie, m_invalid_blocks). The caller must - // lock if it is otherwise needed. + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); LOG_PRINT_L3("Blockchain::" << __func__); if(m_db->block_exists(id)) @@ -2856,7 +2996,11 @@ bool Blockchain::have_block_unlocked(const crypto::hash& id, int *where) const //------------------------------------------------------------------ bool Blockchain::have_block(const crypto::hash& id, int *where) const { - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); return have_block_unlocked(id, where); } //------------------------------------------------------------------ @@ -2870,11 +3014,10 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, block_verification_ size_t Blockchain::get_total_transactions() const { LOG_PRINT_L3("Blockchain::" << __func__); - // WARNING: this function does not take m_blockchain_lock, and thus should only call read only - // m_db functions which do not depend on one another (ie, no getheight + gethash(height-1), as - // well as not accessing class members, even read only (ie, m_invalid_blocks). The caller must - // lock if it is otherwise needed. - return m_db->get_tx_count(); + m_blockchain_transaction.start_read(); + size_t tx_count = m_db->get_tx_count(); + m_blockchain_transaction.end_read(); + return tx_count; } //------------------------------------------------------------------ // This function checks each input in the transaction to make sure it @@ -2886,7 +3029,11 @@ size_t Blockchain::get_total_transactions() const bool Blockchain::check_for_double_spend(const transaction& tx, key_images_container& keys_this_block) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); struct add_transaction_input_visitor: public boost::static_visitor { key_images_container& m_spent_keys; @@ -2946,7 +3093,11 @@ bool Blockchain::check_for_double_spend(const transaction& tx, key_images_contai bool Blockchain::get_tx_outputs_gindexs(const crypto::hash& tx_id, size_t n_txes, std::vector>& indexs) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); uint64_t tx_index; if (!m_db->tx_exists(tx_id, tx_index)) { @@ -2962,7 +3113,11 @@ bool Blockchain::get_tx_outputs_gindexs(const crypto::hash& tx_id, size_t n_txes bool Blockchain::get_tx_outputs_gindexs(const crypto::hash& tx_id, std::vector& indexs) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); uint64_t tx_index; if (!m_db->tx_exists(tx_id, tx_index)) { @@ -3004,7 +3159,11 @@ void Blockchain::on_new_tx_from_block(const cryptonote::transaction &tx) bool Blockchain::check_tx_inputs(transaction& tx, uint64_t& max_used_block_height, crypto::hash& max_used_block_id, tx_verification_context &tvc, bool kept_by_block) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); #if defined(PER_BLOCK_CHECKPOINT) // check if we're doing per-block checkpointing @@ -3035,7 +3194,11 @@ bool Blockchain::check_tx_inputs(transaction& tx, uint64_t& max_used_block_heigh bool Blockchain::check_tx_outputs(const transaction& tx, tx_verification_context &tvc) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); const uint8_t hf_version = m_hardfork->get_current_version(); @@ -3947,7 +4110,11 @@ bool Blockchain::check_tx_input(size_t tx_version, const txin_to_key& txin, cons // ND: // 1. Disable locking and make method private. - //CRITICAL_REGION_LOCAL(m_blockchain_lock); + // m_blockchain_transaction.start_write(); + // epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + // epee::misc_utils::create_scope_leave_handler([&](){ + // m_blockchain_transaction.end_write(); + // }); struct outputs_visitor { @@ -4137,7 +4304,11 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& LOG_PRINT_L3("Blockchain::" << __func__); TIME_MEASURE_START(block_processing_time); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); TIME_MEASURE_START(t1); static bool seen_future_version = false; @@ -4541,7 +4712,11 @@ bool Blockchain::prune_blockchain(uint32_t pruning_seed) { m_tx_pool.lock(); epee::misc_utils::auto_scope_leave_caller unlocker = epee::misc_utils::create_scope_leave_handler([&](){m_tx_pool.unlock();}); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); return m_db->prune_blockchain(pruning_seed); } @@ -4550,7 +4725,11 @@ bool Blockchain::update_blockchain_pruning() { m_tx_pool.lock(); epee::misc_utils::auto_scope_leave_caller unlocker = epee::misc_utils::create_scope_leave_handler([&](){m_tx_pool.unlock();}); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); return m_db->update_pruning(); } @@ -4559,7 +4738,11 @@ bool Blockchain::check_blockchain_pruning() { m_tx_pool.lock(); epee::misc_utils::auto_scope_leave_caller unlocker = epee::misc_utils::create_scope_leave_handler([&](){m_tx_pool.unlock();}); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); return m_db->check_pruning(); } @@ -4663,7 +4846,6 @@ bool Blockchain::add_new_block(const block& bl, block_verification_context& bvc) LOG_PRINT_L3("Blockchain::" << __func__); crypto::hash id = get_block_hash(bl); CRITICAL_REGION_LOCAL(m_tx_pool);//to avoid deadlock lets lock tx_pool for whole add/reorganize process - CRITICAL_REGION_LOCAL1(m_blockchain_lock); db_rtxn_guard rtxn_guard(m_db); if(have_block(id)) { @@ -4679,15 +4861,20 @@ bool Blockchain::add_new_block(const block& bl, block_verification_context& bvc) //chain switching or wrong block bvc.m_added_to_main_chain = false; rtxn_guard.stop(); + m_blockchain_transaction.start_write(); bool r = handle_alternative_block(bl, id, bvc); + m_blockchain_transaction.end_write(); m_blocks_txs_check.clear(); return r; //never relay alternative blocks } rtxn_guard.stop(); - return handle_block_to_main_chain(bl, id, bvc); + m_blockchain_transaction.start_write(); + bool ret_handle_block_to_main_chain = handle_block_to_main_chain(bl, id, bvc); + m_blockchain_transaction.end_write(); + return ret_handle_block_to_main_chain; } catch (const std::exception &e) { @@ -4704,7 +4891,11 @@ void Blockchain::check_against_checkpoints(const checkpoints& points, bool enfor const auto& pts = points.get_points(); bool stop_batch; - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); stop_batch = m_db->batch_start(); const uint64_t blockchain_height = m_db->height(); for (const auto& pt : pts) @@ -4802,7 +4993,7 @@ bool Blockchain::cleanup_handle_incoming_blocks(bool force_sync) bool success = false; MTRACE("Blockchain::" << __func__); - CRITICAL_REGION_BEGIN(m_blockchain_lock); + m_blockchain_transaction.start_write(); TIME_MEASURE_START(t1); try @@ -4866,7 +5057,7 @@ bool Blockchain::cleanup_handle_incoming_blocks(bool force_sync) m_blocks_hash_check.shrink_to_fit(); } - CRITICAL_REGION_END(); + m_blockchain_transaction.end_write(); m_tx_pool.unlock(); update_blockchain_pruning(); @@ -4898,7 +5089,11 @@ uint64_t Blockchain::prevalidate_block_hashes(uint64_t height, const std::vector CHECK_AND_ASSERT_MES(weights.empty() || weights.size() == hashes.size(), 0, "Unexpected weights size"); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); // easy case: height >= hashes if (height >= m_blocks_hash_of_hashes.size() * HASH_OF_HASHES_STEP) @@ -5054,7 +5249,11 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vectorbatch_start(blocks_entry.size(), bytes))) { - m_blockchain_lock.unlock(); + m_blockchain_transaction.end_write(); m_tx_pool.unlock(); epee::misc_utils::sleep_no_w(1000); m_tx_pool.lock(); - m_blockchain_lock.lock(); + m_blockchain_transaction.start_write(); } m_batch_success = true; @@ -5441,7 +5640,11 @@ void Blockchain::add_block_notify(BlockNotifyCallback&& notify) { if (notify) { - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); m_block_notifiers.push_back(std::move(notify)); } } @@ -5450,7 +5653,11 @@ void Blockchain::add_miner_notify(MinerNotifyCallback&& notify) { if (notify) { - CRITICAL_REGION_LOCAL(m_blockchain_lock); + m_blockchain_transaction.start_write(); + epee::misc_utils::auto_scope_leave_caller scope_exit_handler = + epee::misc_utils::create_scope_leave_handler([&](){ + m_blockchain_transaction.end_write(); + }); m_miner_notifiers.push_back(std::move(notify)); } } @@ -5653,12 +5860,12 @@ bool Blockchain::is_within_compiled_block_hash_area(uint64_t height) const void Blockchain::lock() { - m_blockchain_lock.lock(); + m_blockchain_transaction.start_write(); } void Blockchain::unlock() { - m_blockchain_lock.unlock(); + m_blockchain_transaction.end_write(); } bool Blockchain::for_all_key_images(std::function f) const diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index be2848d09fa..0bd16c624bb 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -45,6 +45,7 @@ #include #include #include +#include #include "span.h" #include "syncobj.h" @@ -112,6 +113,93 @@ namespace cryptonote uint64_t already_generated_coins; //!< the total coins minted after that block }; + /** + * @brief serialize the access to blockchain data + */ + struct blockchain_transaction { + + blockchain_transaction() : r_threads(0), w_locks(0) + {}; + + void start_read() { + while (!all_locks_released()) { + w_condition.wait(w_mutex); + } + { + boost::lock_guard lock(_g_mutex); + r_mutex.lock(); + one_more_read_thread(); + } + }; + + void start_write() { + while (!is_zero_thread_reading()) { + r_condition.wait(r_mutex); + } + { + boost::lock_guard lock(_g_mutex); + w_mutex.lock(); + one_more_write_lock(); + } + }; + + void end_read() { + boost::lock_guard lock(_g_mutex); + one_read_thread_finished(); + if(is_zero_thread_reading()) { + r_mutex.unlock(); + r_condition.notify_all(); + } + }; + + void end_write() { + boost::lock_guard lock(_g_mutex); + w_mutex.unlock(); + one_write_lock_released(); + if(all_locks_released()) { + w_condition.notify_all(); + } + }; + + ~blockchain_transaction() { }; + + private: + + inline void one_more_read_thread() { + r_threads.fetch_add(1, std::memory_order::memory_order_seq_cst); + } + + inline void one_read_thread_finished() { + r_threads.fetch_sub(1, std::memory_order::memory_order_seq_cst); + } + + inline bool is_zero_thread_reading() { + return !r_threads.load(std::memory_order::memory_order_seq_cst); + } + + inline void one_more_write_lock() { + w_locks.fetch_add(1, std::memory_order::memory_order_seq_cst); + } + + inline void one_write_lock_released() { + w_locks.fetch_sub(1, std::memory_order::memory_order_seq_cst); + } + + inline bool all_locks_released() { + return !w_locks.load(std::memory_order::memory_order_seq_cst); + } + + boost::mutex _g_mutex; + + boost::shared_mutex r_mutex; + std::atomic r_threads; + boost::condition_variable_any r_condition; + + boost::recursive_mutex w_mutex; + std::atomic w_locks; + boost::condition_variable_any w_condition; + }; + /** * @brief Blockchain destructor */ @@ -1139,7 +1227,7 @@ namespace cryptonote tx_memory_pool& m_tx_pool; - mutable epee::critical_section m_blockchain_lock; // TODO: add here reader/writer lock + mutable blockchain_transaction m_blockchain_transaction; // main chain size_t m_current_block_cumul_weight_limit;