Make sure we won't try to mdb_txn_abort a failing mdb_txn_commit.

Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>

diff --git a/ext/lmdb-safe/lmdb-safe.cc b/ext/lmdb-safe/lmdb-safe.cc
index 24556b3033f90ab509869ad60da008b4a7efc3d9..fc9fbf123d068a40c285b7472c608b29c32378cb 100644 (file)
--- a/ext/lmdb-safe/lmdb-safe.cc
+++ b/ext/lmdb-safe/lmdb-safe.cc
@@ -296,28 +296,28 @@ MDBRWTransactionImpl::~MDBRWTransactionImpl()
 void MDBRWTransactionImpl::commit()
 {
   closeRORWCursors();
-  if (!d_txn) {
-    return;
-  }
-
-  if(int retCode = mdb_txn_commit(d_txn); retCode != 0) {
-    throw std::runtime_error("committing: " + MDBError(retCode));
+  if (d_txn != nullptr) {
+    int retCode = mdb_txn_commit(d_txn);
+    // Upon failure, mdb_txn_commit() performs an mdb_txn_abort() call,
+    // so we need to consider the transaction aborted and correctly
+    // deallocated.
+    d_txn = nullptr;
+    environment().decRWTX();
+    if (retCode != 0) {
+      throw std::runtime_error("committing: " + MDBError(retCode));
+    }
   }
-  environment().decRWTX();
-  d_txn = nullptr;
 }
 
 void MDBRWTransactionImpl::abort()
 {
   closeRORWCursors();
-  if (!d_txn) {
-    return;
+  if (d_txn != nullptr) {
+    mdb_txn_abort(d_txn);
+    d_txn = nullptr;
+    // prevent the RO destructor from cleaning up the transaction itself
+    environment().decRWTX();
   }
-
-  mdb_txn_abort(d_txn);
-  // prevent the RO destructor from cleaning up the transaction itself
-  environment().decRWTX();
-  d_txn = nullptr;
 }
 
 MDBROTransactionImpl::MDBROTransactionImpl(MDBEnv *parent, MDB_txn *txn):
@@ -370,7 +370,7 @@ void MDBROTransactionImpl::abort()
 {
   closeROCursors();
   // if d_txn is non-nullptr here, either the transaction object was invalidated earlier (e.g. by moving from it), or it is an RW transaction which has already cleaned up the d_txn pointer (with an abort).
-  if (d_txn) {
+  if (d_txn != nullptr) {
     mdb_txn_abort(d_txn); // this appears to work better than abort for r/o database opening
     d_txn = nullptr;
   }
@@ -380,7 +380,7 @@ void MDBROTransactionImpl::commit()
 {
   closeROCursors();
   // if d_txn is non-nullptr here, either the transaction object was invalidated earlier (e.g. by moving from it), or it is an RW transaction which has already cleaned up the d_txn pointer (with an abort).
-  if (d_txn) {
+  if (d_txn != nullptr) {
     mdb_txn_commit(d_txn); // this appears to work better than abort for r/o database opening
     d_txn = nullptr;
   }