From 935f74624928ddff8edf5bb32e68d3adfa91ef81 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 29 Oct 2017 13:06:45 +0000 Subject: [PATCH] blockchain: do not lock the blockchain lock for simple DB getters It is safe in those cases, though might return slightly out of date information if another thread is busy modifying the blockchain, but it avoids potentially lengthy delays just to get things like the current blockchain height. --- src/cryptonote_core/blockchain.cpp | 35 ++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 933914363..9d89c6280 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -139,14 +139,20 @@ Blockchain::Blockchain(tx_memory_pool& tx_pool) : bool Blockchain::have_tx(const crypto::hash &id) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + // 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); } //------------------------------------------------------------------ bool Blockchain::have_tx_keyimg_as_spent(const crypto::key_image &key_im) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + // 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); } //------------------------------------------------------------------ @@ -287,7 +293,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__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + // 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(); } //------------------------------------------------------------------ @@ -571,7 +580,10 @@ crypto::hash Blockchain::get_tail_id(uint64_t& height) const crypto::hash Blockchain::get_tail_id() const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + // 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(); } //------------------------------------------------------------------ @@ -633,7 +645,10 @@ 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__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + // 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. try { return m_db->get_block_hash_from_height(height); @@ -1928,7 +1943,10 @@ bool Blockchain::find_blockchain_supplement(const std::list& qbloc uint64_t Blockchain::block_difficulty(uint64_t i) const { LOG_PRINT_L3("Blockchain::" << __func__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + // 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. try { return m_db->get_block_difficulty(i); @@ -2209,7 +2227,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__); - CRITICAL_REGION_LOCAL(m_blockchain_lock); + // 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(); } //------------------------------------------------------------------