From f5581c35364ca71386d4ad4dc0c91cf0617339be Mon Sep 17 00:00:00 2001 From: warptangent Date: Sat, 13 Feb 2016 03:35:48 -0800 Subject: [PATCH 1/4] BlockchainLMDB: Replace remaining txn pointer NULLs with nullptr For consistency. --- src/blockchain_db/lmdb/db_lmdb.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 28e6f552..2d116c53 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -235,7 +235,7 @@ mdb_txn_safe::mdb_txn_safe() : m_txn(NULL) mdb_txn_safe::~mdb_txn_safe() { LOG_PRINT_L3("mdb_txn_safe: destructor"); - if (m_txn != NULL) + if (m_txn != nullptr) { if (m_batch_txn) // this is a batch txn and should have been handled before this point for safety { @@ -265,19 +265,19 @@ void mdb_txn_safe::commit(std::string message) if (auto result = mdb_txn_commit(m_txn)) { - m_txn = NULL; + m_txn = nullptr; throw0(DB_ERROR((message + ": ").append(mdb_strerror(result)).c_str())); } - m_txn = NULL; + m_txn = nullptr; } void mdb_txn_safe::abort() { LOG_PRINT_L3("mdb_txn_safe: abort()"); - if(m_txn != NULL) + if(m_txn != nullptr) { mdb_txn_abort(m_txn); - m_txn = NULL; + m_txn = nullptr; } else { @@ -2216,7 +2216,7 @@ void BlockchainLMDB::block_txn_stop() time_commit1 += time1; delete m_write_txn; - m_write_txn = NULL; + m_write_txn = nullptr; } } @@ -2226,7 +2226,7 @@ void BlockchainLMDB::block_txn_abort() if (! m_batch_active) { delete m_write_txn; - m_write_txn = NULL; + m_write_txn = nullptr; } } @@ -2280,7 +2280,7 @@ void BlockchainLMDB::pop_block(block& blk, std::vector& txs) BlockchainDB::pop_block(blk, txs); if (! m_batch_active) { - m_write_txn = NULL; + m_write_txn = nullptr; txn.commit(); } @@ -2288,7 +2288,7 @@ void BlockchainLMDB::pop_block(block& blk, std::vector& txs) catch (...) { m_num_outputs = num_outputs; - m_write_txn = NULL; + m_write_txn = nullptr; throw; } From 9118d0a44c34ceb79368eae3b785b46f6ddf72b5 Mon Sep 17 00:00:00 2001 From: warptangent Date: Sat, 13 Feb 2016 03:41:22 -0800 Subject: [PATCH 2/4] BlockchainLMDB: Call destructor on allocated txn if setup fails --- src/blockchain_db/lmdb/db_lmdb.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 2d116c53..3b3c5691 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -2114,7 +2114,11 @@ void BlockchainLMDB::batch_start(uint64_t batch_num_blocks) // NOTE: need to make sure it's destroyed properly when done if (auto mdb_res = mdb_txn_begin(m_env, NULL, 0, *m_write_batch_txn)) + { + delete m_write_batch_txn; + m_write_batch_txn = nullptr; throw0(DB_ERROR(lmdb_error("Failed to create a transaction for the db: ", mdb_res).c_str())); + } // indicates this transaction is for batch transactions, but not whether it's // active m_write_batch_txn->m_batch_txn = true; @@ -2201,7 +2205,11 @@ void BlockchainLMDB::block_txn_start() { m_write_txn = new mdb_txn_safe(); if (auto mdb_res = mdb_txn_begin(m_env, NULL, 0, *m_write_txn)) + { + delete m_write_txn; + m_write_txn = nullptr; throw0(DB_ERROR(lmdb_error("Failed to create a transaction for the db: ", mdb_res).c_str())); + } } } From c16cc204d320fb61a462397644ec35c04286182a Mon Sep 17 00:00:00 2001 From: warptangent Date: Sat, 13 Feb 2016 03:50:42 -0800 Subject: [PATCH 3/4] BlockchainLMDB: Add sanity check for inconsistent state This hasn't been known to occur in block-level txn abort, but throw exception if it does. --- src/blockchain_db/lmdb/db_lmdb.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 3b3c5691..7b7af23b 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -2233,8 +2233,19 @@ void BlockchainLMDB::block_txn_abort() LOG_PRINT_L3("BlockchainLMDB::" << __func__); if (! m_batch_active) { - delete m_write_txn; - m_write_txn = nullptr; + if (m_write_txn != nullptr) + { + delete m_write_txn; + m_write_txn = nullptr; + } + else + { + // This would probably mean an earlier exception was caught, but then we + // proceeded further than we should have. + throw0(DB_ERROR((std::string("BlockchainLMDB::") + __func__ + + std::string(": block-level DB transaction abort called when write txn doesn't exist") + ).c_str())); + } } } From 199592355998e801f9a5028e434aa364f4b58d8c Mon Sep 17 00:00:00 2001 From: warptangent Date: Sat, 13 Feb 2016 04:10:27 -0800 Subject: [PATCH 4/4] BlockchainLMDB: Deal with DB exceptions at block level with particularity Add another DB error exception type to distinguish failed txn setup from general use of txn. This keeps the error handling flow the same as before the block-level txn setup changes that moved control up a layer to BlockchainDB. --- src/blockchain_db/blockchain_db.h | 8 ++++++++ src/blockchain_db/lmdb/db_lmdb.cpp | 15 +++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h index 5926f34a..fb1c586e 100644 --- a/src/blockchain_db/blockchain_db.h +++ b/src/blockchain_db/blockchain_db.h @@ -175,6 +175,14 @@ class DB_ERROR : public DB_EXCEPTION DB_ERROR(const char* s) : DB_EXCEPTION(s) { } }; +// For distinguishing errors trying to set up a DB txn from other errors +class DB_ERROR_TXN_START : public DB_EXCEPTION +{ + public: + DB_ERROR_TXN_START() : DB_EXCEPTION("DB Error in starting txn") { } + DB_ERROR_TXN_START(const char* s) : DB_EXCEPTION(s) { } +}; + class DB_OPEN_FAILURE : public DB_EXCEPTION { public: diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index 7b7af23b..0bee8aae 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -2199,8 +2199,15 @@ void BlockchainLMDB::set_batch_transactions(bool batch_transactions) void BlockchainLMDB::block_txn_start() { LOG_PRINT_L3("BlockchainLMDB::" << __func__); + // Distinguish the exceptions here from exceptions that would be thrown while + // using the txn and committing it. + // + // If an exception is thrown in this setup, we don't want the caller to catch + // it and proceed as if there were an existing write txn, such as trying to + // call block_txn_abort(). It also indicates a serious issue which will + // probably be thrown up another layer. if (! m_batch_active && m_write_txn) - throw0(DB_ERROR((std::string("Attempted to start new write txn when write txn already exists in ")+__FUNCTION__).c_str())); + throw0(DB_ERROR_TXN_START((std::string("Attempted to start new write txn when write txn already exists in ")+__FUNCTION__).c_str())); if (! m_batch_active) { m_write_txn = new mdb_txn_safe(); @@ -2208,7 +2215,7 @@ void BlockchainLMDB::block_txn_start() { delete m_write_txn; m_write_txn = nullptr; - throw0(DB_ERROR(lmdb_error("Failed to create a transaction for the db: ", mdb_res).c_str())); + throw0(DB_ERROR_TXN_START(lmdb_error("Failed to create a transaction for the db: ", mdb_res).c_str())); } } } @@ -2270,6 +2277,10 @@ uint64_t BlockchainLMDB::add_block(const block& blk, const size_t& block_size, c { BlockchainDB::add_block(blk, block_size, cumulative_difficulty, coins_generated, txs); } + catch (DB_ERROR_TXN_START& e) + { + throw; + } catch (...) { m_num_outputs = num_outputs;