lmdb: be sure to abort pending transactions in the correct order.

If the LMDBBackend destructor is invoked while there are still pending
transactions, these need to be aborted, but in the reverse order of
their creation (i.e. abort the innermost transaction first).

The default destructor would abort them in a class field
declaration-dependent order, which may not match the actual cinematic.

We now remember which transaction is the innermost one, so that we can
abort them in the expected order.

This gets rid of "double free or corruption (top)" aborts with glibc,
and Address Sanitizer errors.

diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc
index bb701e628f9a7468b9f3b8b4d839707418d0430a..b73fcc04a748afb7212661210b8a20adda3ea683 100644 (file)
--- a/modules/lmdbbackend/lmdbbackend.cc
+++ b/modules/lmdbbackend/lmdbbackend.cc
@@ -809,6 +809,23 @@ LMDBBackend::LMDBBackend(const std::string& suffix)
   d_dolog = ::arg().mustDo("query-logging");
 }
 
+LMDBBackend::~LMDBBackend()
+{
+  // LMDB internals require that, if we have multiple transactions active,
+  // we destroy them in the reverse order of their creation, thus we can't
+  // let the default destructor take care of d_rotxn and d_rwtxn.
+  if (d_txnorder) {
+    // RO transaction more recent than RW transaction
+    d_rotxn.reset();
+    d_rwtxn.reset();
+  }
+  else {
+    // RW transaction more recent than RO transaction
+    d_rwtxn.reset();
+    d_rotxn.reset();
+  }
+}
+
 namespace boost
 {
 namespace serialization
@@ -1073,6 +1090,7 @@ bool LMDBBackend::startTransaction(const DNSName& domain, int domain_id)
     throw DBException("Attempt to start a transaction while one was open already");
   }
   d_rwtxn = getRecordsRWTransaction(real_id);
+  d_txnorder = false;
 
   d_transactiondomain = domain;
   d_transactiondomainid = real_id;
@@ -1443,6 +1461,7 @@ bool LMDBBackend::list(const DNSName& target, int /* id */, bool include_disable
   }
 
   d_rotxn = getRecordsROTransaction(di.id, d_rwtxn);
+  d_txnorder = true;
   d_getcursor = std::make_shared<MDBROCursor>(d_rotxn->txn->getCursor(d_rotxn->db->dbi));
 
   compoundOrdername co;
@@ -1503,6 +1522,7 @@ void LMDBBackend::lookup(const QType& type, const DNSName& qdomain, int zoneId,
   }
   // cout<<"get will look for "<<relqname<< " in zone "<<hunt<<" with id "<<zoneId<<" and type "<<type.toString()<<endl;
   d_rotxn = getRecordsROTransaction(zoneId, d_rwtxn);
+  d_txnorder = true;
 
   compoundOrdername co;
   d_getcursor = std::make_shared<MDBROCursor>(d_rotxn->txn->getCursor(d_rotxn->db->dbi));

diff --git a/modules/lmdbbackend/lmdbbackend.hh b/modules/lmdbbackend/lmdbbackend.hh
index 0b40b5284be5e2ebecb33ada7444e609549f2e6e..523f24871186d3ac081d181ce209d4c997507320 100644 (file)
--- a/modules/lmdbbackend/lmdbbackend.hh
+++ b/modules/lmdbbackend/lmdbbackend.hh
@@ -60,6 +60,7 @@ class LMDBBackend : public DNSBackend
 {
 public:
   explicit LMDBBackend(const string& suffix = "");
+  ~LMDBBackend();
 
   bool list(const DNSName& target, int id, bool include_disabled) override;
 
@@ -305,6 +306,7 @@ private:
 
   shared_ptr<RecordsROTransaction> d_rotxn; // for lookup and list
   shared_ptr<RecordsRWTransaction> d_rwtxn; // for feedrecord within begin/aborttransaction
+  bool d_txnorder{false}; // whether d_rotxn is more recent than d_rwtxn
   std::shared_ptr<RecordsRWTransaction> getRecordsRWTransaction(uint32_t id);
   std::shared_ptr<RecordsROTransaction> getRecordsROTransaction(uint32_t id, const std::shared_ptr<LMDBBackend::RecordsRWTransaction>& rwtxn = nullptr);
   int genChangeDomain(const DNSName& domain, const std::function<void(DomainInfo&)>& func);