Hide MDB*Transaction behind a unique_ptr front

This is to prevent the issue with Object Slicing. With the previous
solution (where MDB*Transaction are normal objects), consider the
following code:

    MDBRWTransaction txn = env.getRWTransaction();

    //! Invalid: We explicitly break this move because it would be
    //! unsafe:
    // MDBROTransaction ro_txn(std::move(txn));

    //! Valid, RW inherits from RO now, so we can bind an RO
    //! reference to an RW transaction.
    MDBROTransaction &ro_txn = txn;

    //! Dangerous!!
    MDBROTransaction ro_txn2(std::move(ro_txn));

The last move there breaks the semantics of the RW transaction which
is bound to the reference ro_txn. It looses its RW cursors, which
remain partly inside the txn instance. All kinds of weird and bad
things can happen here. For instance, the ro_txn2 would go out of
scope before the txn, calling the destructor MDBROTransaction
destructor (which defaults to commit instead of abort!) and only
freeing parts of the cursors. Only then the MDBRWTransaction
destructor is called, which will free the cursors which belong to
the RW transaction which has already been committed.

The only safe way to prevent Object Slicing in this scenario I
could come up with is to disallow moves of the objects altogether
and instead use unique_ptr as front for them. This also removes
an additional dynamic allocation per RW transaction (for the
cursor vector), since the address of that vector is now constant
over the lifetime of the transaction without indirection.

README.md

@@ -87,16 +87,16 @@ transaction is aborted automatically. To commit or abort, use `commit()` or
 `abort()`, after which going out of scope has no further effect.
 
 ```
-  txn.put(dbi, "lmdb", "great");
+  txn->put(dbi, "lmdb", "great");
 
   string_view data;
-  if(!txn.get(dbi, "lmdb", data)) {
+  if(!txn->get(dbi, "lmdb", data)) {
     cout<< "Within RW transaction, found that lmdb = " << data <<endl;
   }
   else
     cout<<"Found nothing" << endl;
 
-  txn.commit();
+  txn->commit();
 ```
 
 LMDB is so fast because it does not copy data unless it really needs to.
@@ -129,8 +129,8 @@ For example, to store `double` values for 64 bit IDs:
   auto txn = env->getRWTransaction();
   uint64_t id=12345678901;
   double score=3.14159;
-  txn.put(dbi, id, score);
+  txn->put(dbi, id, score);
-  txn.commit();
+  txn->commit();
 ```
 
 Behind the scenes, the `id` and `score` values are wrapped by `MDBInVal`
@@ -142,7 +142,7 @@ works similary:
   uint64_t id=12345678901;
   MDBOutValue val;
 
-  txn.get(dbi, id, val);
+  txn->get(dbi, id, val);
 
   cout << "Score: " << val.get<double>() << "\n";
 ```
@@ -170,10 +170,10 @@ struct Coordinate
 
 C c{12.0, 13.0};
 
-txn.put(dbi, MDBInVal::fromStruct(c), 12.0);
+txn->put(dbi, MDBInVal::fromStruct(c), 12.0);
 
 MDBOutVal res;
-txn.get(dbi, MDBInVal::fromStruct(c), res);
+txn->get(dbi, MDBInVal::fromStruct(c), res);
 
 auto c1 = res.get_struct<Coordinate>();
 ```
@@ -193,7 +193,7 @@ calls to mdb.
 This is the usual opening sequence.
 
 ```
-  auto cursor=txn.getCursor(dbi);
+  auto cursor=txn->getCursor(dbi);
   MDBOutVal key, data;
   int count=0;
   cout<<"Counting records.. "; cout.flush();
@@ -212,7 +212,7 @@ records in under a second (!).
   
 ```
   cout<<"Clearing records.. "; cout.flush();
-  mdb_drop(txn, dbi, 0); // clear records
+  mdb_drop(*txn, dbi, 0); // clear records
   cout<<"Done!"<<endl;
 ```
 
@@ -224,11 +224,11 @@ native `mdb_drop` function which we did not wrap. This is possible because
 ```
   cout << "Adding "<<limit<<" values  .. "; cout.flush();
   for(unsigned int n = 0 ; n < limit; ++n) {
-    txn.put(dbi, n, n);
+    txn->put(dbi, n, n);
   }
   cout <<"Done!"<<endl;
   cout <<"Calling commit.. "; cout.flush();
-  txn.commit();
+  txn->commit();
   cout<<"Done!"<<endl;
 ```
 

basic-example.cc

@@ -5,7 +5,7 @@ void checkLMDB(MDBEnv* env, MDBDbi dbi)
 {
   auto rotxn = env->getROTransaction();
   MDBOutVal data;
-  if(!rotxn.get(dbi, "lmdb", data)) {
+  if(!rotxn->get(dbi, "lmdb", data)) {
     cout<< "Outside RW transaction, found that lmdb = " << data.get<string_view>() <<endl;
   }
   else
@@ -18,11 +18,11 @@ int main()
   auto dbi = env->openDB("example", MDB_CREATE);
   
   auto txn = env->getRWTransaction();
-  mdb_drop(txn, dbi, 0);
+  mdb_drop(*txn, dbi, 0);
-  txn.put(dbi, "lmdb", "great");
+  txn->put(dbi, "lmdb", "great");
 
   MDBOutVal data;
-  if(!txn.get(dbi, "lmdb", data)) {
+  if(!txn->get(dbi, "lmdb", data)) {
     cout<< "Within RW transaction, found that lmdb = " << data.get<string_view>() <<endl;
   }
   else
@@ -31,12 +31,12 @@ int main()
   std::thread elsewhere(checkLMDB, env.get(), dbi);
   elsewhere.join();
   
-  txn.commit();
+  txn->commit();
   
   cout<<"Committed data"<<endl;
   
   checkLMDB(env.get(), dbi);
   txn = env->getRWTransaction();
-  mdb_drop(txn, dbi, 0);
+  mdb_drop(*txn, dbi, 0);
-  txn.commit();
+  txn->commit();
 }

lmdb-safe.cc

@@ -139,20 +139,20 @@ MDBDbi MDBEnv::openDB(const string_view dbname, int flags)
   
   if(!(envflags & MDB_RDONLY)) {
     auto rwt = getRWTransaction();
-    MDBDbi ret = rwt.openDB(dbname, flags);
+    MDBDbi ret = rwt->openDB(dbname, flags);
-    rwt.commit();
+    rwt->commit();
     return ret;
   }
 
   MDBDbi ret;
   {
     auto rwt = getROTransaction(); 
-    ret = rwt.openDB(dbname, flags);
+    ret = rwt->openDB(dbname, flags);
   }
   return ret;
 }
 
-MDB_txn *MDBRWTransaction::openRWTransaction(MDBEnv *env, MDB_txn *parent, int flags)
+MDB_txn *MDBRWTransactionImpl::openRWTransaction(MDBEnv *env, MDB_txn *parent, int flags)
 {
   MDB_txn *result;
   if(env->getROTX() || env->getRWTX())
@@ -174,18 +174,18 @@ MDB_txn *MDBRWTransaction::openRWTransaction(MDBEnv *env, MDB_txn *parent, int f
   return result;
 }
 
-MDBRWTransaction::MDBRWTransaction(MDBEnv* parent, int flags):
+MDBRWTransactionImpl::MDBRWTransactionImpl(MDBEnv* parent, int flags):
-  MDBROTransaction(parent, openRWTransaction(parent, nullptr, flags)),
+  MDBROTransactionImpl(parent, openRWTransaction(parent, nullptr, flags)),
-  d_rw_cursors(new decltype(d_rw_cursors)::element_type())
+  d_rw_cursors()
 {
 }
 
-MDBRWTransaction::~MDBRWTransaction()
+MDBRWTransactionImpl::~MDBRWTransactionImpl()
 {
   abort();
 }
 
-void MDBRWTransaction::commit()
+void MDBRWTransactionImpl::commit()
 {
   closeRORWCursors();
   if (!d_txn) {
@@ -199,7 +199,7 @@ void MDBRWTransaction::commit()
   d_txn = nullptr;
 }
 
-void MDBRWTransaction::abort()
+void MDBRWTransactionImpl::abort()
 {
   closeRORWCursors();
   if (!d_txn) {
@@ -212,15 +212,15 @@ void MDBRWTransaction::abort()
   d_txn = nullptr;
 }
 
-MDBROTransaction::MDBROTransaction(MDBEnv *parent, MDB_txn *txn):
+MDBROTransactionImpl::MDBROTransactionImpl(MDBEnv *parent, MDB_txn *txn):
   d_parent(parent),
-  d_cursors(new decltype(d_cursors)::element_type()),
+  d_cursors(),
   d_txn(txn)
 {
 
 }
 
-MDB_txn *MDBROTransaction::openROTransaction(MDBEnv *env, MDB_txn *parent, int flags)
+MDB_txn *MDBROTransactionImpl::openROTransaction(MDBEnv *env, MDB_txn *parent, int flags)
 {
   if(env->getRWTX())
     throw std::runtime_error("Duplicate RO transaction");
@@ -246,32 +246,29 @@ MDB_txn *MDBROTransaction::openROTransaction(MDBEnv *env, MDB_txn *parent, int f
   return result;
 }
 
-void MDBROTransaction::closeROCursors()
+void MDBROTransactionImpl::closeROCursors()
 {
-  if (!d_cursors) {
-    return;
-  }
   // we need to move the vector away to ensure that the cursors don’t mess with our iteration.
   std::vector<MDBROCursor*> buf;
-  std::swap(*d_cursors, buf);
+  std::swap(d_cursors, buf);
   for (auto &cursor: buf) {
     cursor->close();
   }
 }
 
-MDBROTransaction::MDBROTransaction(MDBEnv *parent, int flags):
+MDBROTransactionImpl::MDBROTransactionImpl(MDBEnv *parent, int flags):
-    MDBROTransaction(parent, openROTransaction(parent, nullptr, flags))
+    MDBROTransactionImpl(parent, openROTransaction(parent, nullptr, flags))
 {
 
 }
 
-MDBROTransaction::~MDBROTransaction()
+MDBROTransactionImpl::~MDBROTransactionImpl()
 {
   // this is safe because C++ will not call overrides of virtual methods in destructors.
   commit();
 }
 
-void MDBROTransaction::abort()
+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).
@@ -282,7 +279,7 @@ void MDBROTransaction::abort()
   }
 }
 
-void MDBROTransaction::commit()
+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).
@@ -295,63 +292,60 @@ void MDBROTransaction::commit()
 
 
 
-void MDBRWTransaction::clear(MDB_dbi dbi)
+void MDBRWTransactionImpl::clear(MDB_dbi dbi)
 {
   if(int rc = mdb_drop(d_txn, dbi, 0)) {
     throw runtime_error("Error clearing database: " + MDBError(rc));
   }
 }
 
-MDBRWCursor MDBRWTransaction::getRWCursor(const MDBDbi& dbi)
+MDBRWCursor MDBRWTransactionImpl::getRWCursor(const MDBDbi& dbi)
 {
   MDB_cursor *cursor;
   int rc= mdb_cursor_open(d_txn, dbi, &cursor);
   if(rc) {
     throw std::runtime_error("Error creating RO cursor: "+std::string(mdb_strerror(rc)));
   }
-  return MDBRWCursor(*d_rw_cursors, cursor);
+  return MDBRWCursor(d_rw_cursors, cursor);
 }
 
-MDBRWCursor MDBRWTransaction::getCursor(const MDBDbi &dbi)
+MDBRWCursor MDBRWTransactionImpl::getCursor(const MDBDbi &dbi)
 {
   return getRWCursor(dbi);
 }
 
 MDBROTransaction MDBEnv::getROTransaction()
 {
-  return MDBROTransaction(this);
+  return MDBROTransaction(new MDBROTransactionImpl(this));
 }
 MDBRWTransaction MDBEnv::getRWTransaction()
 {
-  return MDBRWTransaction(this);
+  return MDBRWTransaction(new MDBRWTransactionImpl(this));
 }
 
 
-void MDBRWTransaction::closeRWCursors()
+void MDBRWTransactionImpl::closeRWCursors()
 {
-  if (!d_rw_cursors) {
+  decltype(d_rw_cursors) buf;
-    return;
+  std::swap(d_rw_cursors, buf);
-  }
-  decltype(d_rw_cursors)::element_type buf;
-  std::swap(*d_rw_cursors, buf);
   for (auto &cursor: buf) {
     cursor->close();
   }
 }
 
-MDBROCursor MDBROTransaction::getCursor(const MDBDbi& dbi)
+MDBROCursor MDBROTransactionImpl::getCursor(const MDBDbi& dbi)
 {
   return getROCursor(dbi);
 }
 
-MDBROCursor MDBROTransaction::getROCursor(const MDBDbi &dbi)
+MDBROCursor MDBROTransactionImpl::getROCursor(const MDBDbi &dbi)
 {
   MDB_cursor *cursor;
   int rc= mdb_cursor_open(d_txn, dbi, &cursor);
   if(rc) {
     throw std::runtime_error("Error creating RO cursor: "+std::string(mdb_strerror(rc)));
   }
-  return MDBROCursor(*d_cursors, cursor);
+  return MDBROCursor(d_cursors, cursor);
 }
 
 

lmdb-safe.hh

@@ -59,8 +59,11 @@ public:
   MDB_dbi d_dbi;
 };
 
-class MDBRWTransaction;
+class MDBRWTransactionImpl;
-class MDBROTransaction;
+class MDBROTransactionImpl;
+
+using MDBROTransaction = std::unique_ptr<MDBROTransactionImpl>;
+using MDBRWTransaction = std::unique_ptr<MDBRWTransactionImpl>;
 
 class MDBEnv
 {
@@ -222,16 +225,16 @@ private:
 
 class MDBROCursor;
 
-class MDBROTransaction
+class MDBROTransactionImpl
 {
 protected:
-  MDBROTransaction(MDBEnv *parent, MDB_txn *txn);
+  MDBROTransactionImpl(MDBEnv *parent, MDB_txn *txn);
 
 private:
   static MDB_txn *openROTransaction(MDBEnv *env, MDB_txn *parent, int flags=0);
 
   MDBEnv* d_parent;
-  std::unique_ptr<std::vector<MDBROCursor*>> d_cursors;
+  std::vector<MDBROCursor*> d_cursors;
 
 protected:
   MDB_txn* d_txn;
@@ -239,39 +242,16 @@ protected:
   void closeROCursors();
 
 public:
-  explicit MDBROTransaction(MDBEnv* parent, int flags=0);
+  explicit MDBROTransactionImpl(MDBEnv* parent, int flags=0);
 
-  MDBROTransaction(const MDBROTransaction& src) = delete;
+  MDBROTransactionImpl(const MDBROTransactionImpl& src) = delete;
-  MDBROTransaction &operator=(const MDBROTransaction& src) = delete;
+  MDBROTransactionImpl &operator=(const MDBROTransactionImpl& src) = delete;
 
-  MDBROTransaction(MDBROTransaction&& rhs) noexcept:
+  // The move constructor/operator cannot be made safe due to Object Slicing with MDBRWTransaction.
-    d_parent(rhs.d_parent),
+  MDBROTransactionImpl(MDBROTransactionImpl&& rhs) = delete;
-    d_cursors(std::move(rhs.d_cursors)),
+  MDBROTransactionImpl &operator=(MDBROTransactionImpl &&rhs) = delete;
-    d_txn(rhs.d_txn)
-  {
-    rhs.d_parent = nullptr;
-    rhs.d_txn = nullptr;
-  }
 
-  MDBROTransaction &operator=(MDBROTransaction &&rhs) noexcept
+  virtual ~MDBROTransactionImpl();
-  {
-    if (d_txn) {
-      abort();
-    }
-    d_parent = rhs.d_parent;
-    d_txn = rhs.d_txn;
-    d_cursors = std::move(d_cursors);
-    rhs.d_txn = nullptr;
-    rhs.d_parent = nullptr;
-    return *this;
-  }
-
-  /* ensure that we cannot move from subclasses, because that would be massively
-   * unsafe. */
-  template<typename T, typename _ = typename std::enable_if<std::is_base_of<MDBROTransaction, T>::value>::type>
-  MDBROTransaction(T&& rhs) = delete;
-
-  virtual ~MDBROTransaction();
 
   virtual void abort();
   virtual void commit();
@@ -337,6 +317,13 @@ private:
   MDB_cursor* d_cursor;
 
 public:
+  MDBGenCursor():
+    d_registry(nullptr),
+    d_cursor(nullptr)
+  {
+
+  }
+
   MDBGenCursor(std::vector<T*> &registry, MDB_cursor *cursor):
     d_registry(&registry),
     d_cursor(cursor)
@@ -347,6 +334,10 @@ public:
 private:
   void move_from(MDBGenCursor *src)
   {
+    if (!d_registry) {
+      return;
+    }
+
     auto iter = std::find(d_registry->begin(),
                           d_registry->end(),
                           src);
@@ -482,10 +473,11 @@ public:
   }
 };
 
-class MDBROCursor : public MDBGenCursor<MDBROTransaction, MDBROCursor>
+class MDBROCursor : public MDBGenCursor<MDBROTransactionImpl, MDBROCursor>
 {
 public:
-  using MDBGenCursor<MDBROTransaction, MDBROCursor>::MDBGenCursor;
+  MDBROCursor() = default;
+  using MDBGenCursor<MDBROTransactionImpl, MDBROCursor>::MDBGenCursor;
   MDBROCursor(const MDBROCursor &src) = delete;
   MDBROCursor(MDBROCursor &&src) = default;
   MDBROCursor &operator=(const MDBROCursor &src) = delete;
@@ -496,13 +488,13 @@ public:
 
 class MDBRWCursor;
 
-class MDBRWTransaction: public MDBROTransaction
+class MDBRWTransactionImpl: public MDBROTransactionImpl
 {
 private:
   static MDB_txn *openRWTransaction(MDBEnv* env, MDB_txn *parent, int flags);
 
 private:
-  std::unique_ptr<std::vector<MDBRWCursor*>> d_rw_cursors;
+  std::vector<MDBRWCursor*> d_rw_cursors;
 
   void closeRWCursors();
   inline void closeRORWCursors() {
@@ -511,23 +503,14 @@ private:
   }
 
 public:
-  explicit MDBRWTransaction(MDBEnv* parent, int flags=0);
+  explicit MDBRWTransactionImpl(MDBEnv* parent, int flags=0);
 
-  MDBRWTransaction(MDBRWTransaction&& rhs) noexcept:
+  MDBRWTransactionImpl(const MDBRWTransactionImpl& rhs) = delete;
-    MDBROTransaction(std::move(static_cast<MDBROTransaction&>(rhs))),
+  MDBRWTransactionImpl(MDBRWTransactionImpl&& rhs) = delete;
-    d_rw_cursors(std::move(rhs.d_rw_cursors))
+  MDBRWTransactionImpl &operator=(const MDBRWTransactionImpl& rhs) = delete;
-  {
+  MDBRWTransactionImpl &operator=(MDBRWTransactionImpl&& rhs) = delete;
 
-  }
+  ~MDBRWTransactionImpl() override;
-
-  MDBRWTransaction &operator=(MDBRWTransaction&& rhs) noexcept
-  {
-    MDBROTransaction::operator=(std::move(static_cast<MDBROTransaction&>(rhs)));
-    d_rw_cursors = std::move(rhs.d_rw_cursors);
-    return *this;
-  }
-
-  ~MDBRWTransaction() override;
   
   void commit() override;
   void abort() override;
@@ -598,13 +581,14 @@ public:
 /* "A cursor in a write-transaction can be closed before its transaction ends, and will otherwise be closed when its transaction ends" 
    This is a problem for us since it may means we are closing the cursor twice, which is bad
 */
-class MDBRWCursor : public MDBGenCursor<MDBRWTransaction, MDBRWCursor>
+class MDBRWCursor : public MDBGenCursor<MDBRWTransactionImpl, MDBRWCursor>
 {
 public:
-  using MDBGenCursor<MDBRWTransaction, MDBRWCursor>::MDBGenCursor;
+  MDBRWCursor() = default;
-  MDBRWCursor(const MDBRWCursor &src) = default;
+  using MDBGenCursor<MDBRWTransactionImpl, MDBRWCursor>::MDBGenCursor;
+  MDBRWCursor(const MDBRWCursor &src) = delete;
   MDBRWCursor(MDBRWCursor &&src) = default;
-  MDBRWCursor &operator=(const MDBRWCursor &src) = default;
+  MDBRWCursor &operator=(const MDBRWCursor &src) = delete;
   MDBRWCursor &operator=(MDBRWCursor &&src) = default;
   ~MDBRWCursor() = default;
 

lmdb-typed.cc

@@ -2,7 +2,7 @@
 
 unsigned int MDBGetMaxID(MDBRWTransaction& txn, MDBDbi& dbi)
 {
-  auto cursor = txn.getRWCursor(dbi);
+  auto cursor = txn->getRWCursor(dbi);
   MDBOutVal maxidval, maxcontent;
   unsigned int maxid{0};
   if(!cursor.get(maxidval, maxcontent, MDB_LAST)) {

lmdb-typed.hh

@@ -101,12 +101,12 @@ struct LMDBIndexOps
   explicit LMDBIndexOps(Parent* parent) : d_parent(parent){}
   void put(MDBRWTransaction& txn, const Class& t, uint32_t id, int flags=0)
   {
-    txn.put(d_idx, keyConv(d_parent->getMember(t)), id, flags);
+    txn->put(d_idx, keyConv(d_parent->getMember(t)), id, flags);
   }
 
   void del(MDBRWTransaction& txn, const Class& t, uint32_t id)
   {
-    if(int rc = txn.del(d_idx, keyConv(d_parent->getMember(t)), id)) {
+    if(int rc = txn->del(d_idx, keyConv(d_parent->getMember(t)), id)) {
       throw std::runtime_error("Error deleting from index: " + std::string(mdb_strerror(rc)));
     }
   }
@@ -205,7 +205,7 @@ public:
     uint32_t size()
     {
       MDB_stat stat;
-      mdb_stat(*d_parent.d_txn, d_parent.d_parent->d_main, &stat);
+      mdb_stat(**d_parent.d_txn, d_parent.d_parent->d_main, &stat);
       return stat.ms_entries;
     }
 
@@ -214,7 +214,7 @@ public:
     uint32_t size()
     {
       MDB_stat stat;
-      mdb_stat(*d_parent.d_txn, std::get<N>(d_parent.d_parent->d_tuple).d_idx, &stat);
+      mdb_stat(**d_parent.d_txn, std::get<N>(d_parent.d_parent->d_tuple).d_idx, &stat);
       return stat.ms_entries;
     }
 
@@ -222,7 +222,7 @@ public:
     bool get(uint32_t id, T& t)
     {
       MDBOutVal data;
-      if(d_parent.d_txn->get(d_parent.d_parent->d_main, id, data))
+      if((*d_parent.d_txn)->get(d_parent.d_parent->d_main, id, data))
         return false;
       
       serFromString(data.get<std::string>(), t);
@@ -234,7 +234,7 @@ public:
     uint32_t get(const typename std::tuple_element<N, tuple_t>::type::type& key, T& out)
     {
       MDBOutVal id;
-      if(!d_parent.d_txn->get(std::get<N>(d_parent.d_parent->d_tuple).d_idx, keyConv(key), id)) {
+      if(!(*d_parent.d_txn)->get(std::get<N>(d_parent.d_parent->d_tuple).d_idx, keyConv(key), id)) {
         if(get(id.get<uint32_t>(), out))
           return id.get<uint32_t>();
       }
@@ -245,7 +245,7 @@ public:
     template<int N>
     uint32_t cardinality()
     {
-      auto cursor = d_parent.d_txn->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
+      auto cursor = (*d_parent.d_txn)->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
       bool first = true;
       MDBOutVal key, data;
       uint32_t count = 0;
@@ -284,7 +284,7 @@ public:
         }
 
         if(d_on_index) {
-          if(d_parent->d_txn->get(d_parent->d_parent->d_main, d_id, d_data))
+          if((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, d_data))
             throw std::runtime_error("Missing id in constructor");
           serFromString(d_data.get<std::string>(), d_t);
         }
@@ -309,7 +309,7 @@ public:
         }
 
         if(d_on_index) {
-          if(d_parent->d_txn->get(d_parent->d_parent->d_main, d_id, d_data))
+          if((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, d_data))
             throw std::runtime_error("Missing id in constructor");
           serFromString(d_data.get<std::string>(), d_t);
         }
@@ -362,7 +362,7 @@ public:
         }
         else {
           if(d_on_index) {
-            if(d_parent->d_txn->get(d_parent->d_parent->d_main, d_id, data))
+            if((*d_parent->d_txn)->get(d_parent->d_parent->d_main, d_id, data))
               throw std::runtime_error("Missing id field");
             if(filter && !filter(data))
               goto next;
@@ -419,7 +419,7 @@ public:
     template<int N>
     iter_t genbegin(MDB_cursor_op op)
     {
-      typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
+      typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
       
       MDBOutVal out, id;
       
@@ -445,7 +445,7 @@ public:
 
     iter_t begin()
     {
-      typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(d_parent.d_parent->d_main);
+      typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(d_parent.d_parent->d_main);
       
       MDBOutVal out, id;
       
@@ -466,7 +466,7 @@ public:
     template<int N>
     iter_t genfind(const typename std::tuple_element<N, tuple_t>::type::type& key, MDB_cursor_op op)
     {
-      typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
+      typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
 
       std::string keystr = keyConv(key);
       MDBInVal in(keystr);
@@ -498,7 +498,7 @@ public:
     template<int N>
     std::pair<iter_t,eiter_t> equal_range(const typename std::tuple_element<N, tuple_t>::type::type& key)
     {
-      typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
+      typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
 
       std::string keyString=keyConv(key);
       MDBInVal in(keyString);
@@ -517,7 +517,7 @@ public:
     template<int N>
     std::pair<iter_t,eiter_t> prefix_range(const typename std::tuple_element<N, tuple_t>::type::type& key)
     {
-      typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
+      typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get<N>(d_parent.d_parent->d_tuple).d_idx);
 
       std::string keyString=keyConv(key);
       MDBInVal in(keyString);
@@ -595,7 +595,7 @@ public:
         id = MDBGetMaxID(*d_txn, d_parent->d_main) + 1;
         flags = MDB_APPEND;
       }
-      d_txn->put(d_parent->d_main, id, serToString(t), flags);
+      (*d_txn)->put(d_parent->d_main, id, serToString(t), flags);
 
 #define insertMacro(N) std::get<N>(d_parent->d_tuple).put(*d_txn, t, id);
       insertMacro(0);
@@ -626,14 +626,14 @@ public:
       if(!this->get(id, t)) 
         return;
       
-      d_txn->del(d_parent->d_main, id);
+      (*d_txn)->del(d_parent->d_main, id);
       clearIndex(id, t);
     }
 
     //! clear database & indexes (by hand!)
     void clear()
     {
-      auto cursor = d_txn->getRWCursor(d_parent->d_main);
+      auto cursor = (*d_txn)->getRWCursor(d_parent->d_main);
       bool first = true;
       MDBOutVal key, data;
       while(!cursor.get(key, data, first ? MDB_FIRST : MDB_NEXT)) {
@@ -648,13 +648,13 @@ public:
     //! commit this transaction
     void commit()
     {
-      d_txn->commit();
+      (*d_txn)->commit();
     }
 
     //! abort this transaction
     void abort()
     {
-      d_txn->abort();
+      (*d_txn)->abort();
     }
 
     typedef MDBRWCursor cursor_t;

lmdb-various.cc

@@ -18,7 +18,7 @@ static void closeTest()
 
   auto txn = env->getROTransaction();
   for(auto& d : {&main, &dbi, &hyc}) {
-    auto rocursor = txn.getCursor(*d);
+    auto rocursor = txn->getCursor(*d);
     MDBOutVal key, data;
     if(rocursor.get(key, data, MDB_FIRST))
       continue;
@@ -41,8 +41,8 @@ try
   for(int n=0; n < 15; ++n) {
     auto txn = env->getRWTransaction();
     int val = n + 1000*tid;
-    txn.put(dbi, val, val);
+    txn->put(dbi, val, val);
-    txn.commit();
+    txn->commit();
     cout << "Done with transaction "<<n<<" in thread " << tid<<endl;
   }
   cout<<"Done with thread "<<tid<<endl;
@@ -62,7 +62,7 @@ try
     auto txn = env->getROTransaction();
     int val = n + 1000*tid;
     MDBOutVal res;
-    if(txn.get(dbi, val, res)) {
+    if(txn->get(dbi, val, res)) {
       throw std::runtime_error("no record");
     }
     
@@ -85,9 +85,9 @@ void doFill()
     auto txn = env->getRWTransaction();
     for(int j=0; j < 1000000; ++j) {
       MDBInVal mv(n*1000000+j);
-      txn.put(dbi, mv, mv, 0);
+      txn->put(dbi, mv, mv, 0);
     }
-    txn.commit();
+    txn->commit();
   }
   cout<<"Done filling"<<endl;
 }
@@ -104,7 +104,7 @@ void doMeasure()
       for(int j=0; j < 1000000; ++j) {
         MDBInVal mv(n*1000000+j);
         MDBOutVal res;
-        if(!txn.get(dbi, mv, res))
+        if(!txn->get(dbi, mv, res))
           ++count;
       }
       cout<<count<<" ";

lmdb-view.cc

@@ -5,8 +5,8 @@ using namespace std;
 
 void countDB(MDBEnv& env, MDBROTransaction& txn, const std::string& dbname)
 {
-  auto db = txn.openDB(dbname, 0);
+  auto db = txn->openDB(dbname, 0);
-  auto cursor = txn.getCursor(db);
+  auto cursor = txn->getCursor(db);
   uint32_t count = 0;
   MDBOutVal key, val;
   while(!cursor.get(key, val, count ? MDB_NEXT : MDB_FIRST)) {
@@ -27,7 +27,7 @@ int main(int argc, char** argv)
   auto main = env.openDB("", 0);
   auto txn = env.getROTransaction();
 
-  auto cursor = txn.getCursor(main);
+  auto cursor = txn->getCursor(main);
 
   MDBOutVal key, val;
   if(cursor.get(key, val, MDB_FIRST)) {

multi-example.cc

@@ -10,26 +10,26 @@ int main()
   auto dbi = env->openDB("qnames", MDB_DUPSORT | MDB_CREATE);
 
   auto txn = env->getRWTransaction();
-  txn.clear(dbi);
+  txn->clear(dbi);
 
-  txn.put(dbi, "bdb", "old");
+  txn->put(dbi, "bdb", "old");
-  txn.put(dbi, "lmdb", "hot");
+  txn->put(dbi, "lmdb", "hot");
-  txn.put(dbi, "lmdb", "fast");
+  txn->put(dbi, "lmdb", "fast");
-  txn.put(dbi, "lmdb", "zooms");
+  txn->put(dbi, "lmdb", "zooms");
-  txn.put(dbi, "lmdb", "c");
+  txn->put(dbi, "lmdb", "c");
-  txn.put(dbi, "mdb", "old name");
+  txn->put(dbi, "mdb", "old name");
 
   string_view v1;
-  if(!txn.get(dbi, "mdb", v1)) {
+  if(!txn->get(dbi, "mdb", v1)) {
     cout<<v1<<endl;
   }
   else {
     cout << "found nothing" << endl;
   }
-  txn.commit();
+  txn->commit();
   
   txn = env->getRWTransaction();
-  auto cursor = txn.getRWCursor(dbi);
+  auto cursor = txn->getRWCursor(dbi);
 
   MDBOutVal key, data;
 

rel-example.cc

@@ -27,7 +27,7 @@ struct Record
 
 static unsigned int getMaxID(MDBRWTransaction& txn, MDBDbi& dbi)
 {
-  auto cursor = txn.getRWCursor(dbi);
+  auto cursor = txn->getRWCursor(dbi);
   MDBOutVal maxidval, maxcontent;
   unsigned int maxid{0};
   if(!cursor.get(maxidval, maxcontent, MDB_LAST)) {
@@ -42,9 +42,9 @@ static void store(MDBRWTransaction& txn, MDBDbi& records, MDBDbi& domainidx, MDB
   boost::archive::binary_oarchive oa(oss,boost::archive::no_header );
   oa << r;
   
-  txn.put(records, r.id, oss.str(), MDB_APPEND);
+  txn->put(records, r.id, oss.str(), MDB_APPEND);
-  txn.put(domainidx, r.domain_id, r.id);
+  txn->put(domainidx, r.domain_id, r.id);
-  txn.put(nameidx, r.name, r.id);
+  txn->put(nameidx, r.name, r.id);
 }
 
 
@@ -122,12 +122,12 @@ int main(int argc, char** argv)
     store(txn, records, domainidx, nameidx, r);
   }
   
-  txn.commit();
+  txn->commit();
 
   auto rotxn = env->getROTransaction();
   auto rotxn2 = env->getROTransaction();
   
-  auto rocursor = rotxn.getCursor(nameidx);
+  auto rocursor = rotxn->getCursor(nameidx);
 
   MDBOutVal data;
   int count = 0;
@@ -143,7 +143,7 @@ int main(int argc, char** argv)
     cout<<"Got something: id="<<id<<endl;
     MDBOutVal record;
 
-    if(!rotxn.get(records, data, record)) {
+    if(!rotxn->get(records, data, record)) {
       Record test;
       stringstream istr{record.get<string>()};
       boost::archive::binary_iarchive oi(istr,boost::archive::no_header );

resize-example.cc

@@ -10,42 +10,42 @@ int main(int argc, char** argv)
   MDBInVal key("counter");
 
   auto rwtxn = env->getRWTransaction();
-  rwtxn.put(main, "counter", "1234");
+  rwtxn->put(main, "counter", "1234");
-  rwtxn.put(main, MDBInVal::fromStruct(std::make_pair(12,13)), "hoi dan 12,13");
+  rwtxn->put(main, MDBInVal::fromStruct(std::make_pair(12,13)), "hoi dan 12,13");
 
-  rwtxn.put(main, MDBInVal::fromStruct(std::make_pair(14,15)),
+  rwtxn->put(main, MDBInVal::fromStruct(std::make_pair(14,15)),
              MDBInVal::fromStruct(std::make_pair(20,23)));
 
   
   MDBOutVal out;
-  if(!rwtxn.get(main, MDBInVal::fromStruct(std::make_pair(12,13)), out)) 
+  if(!rwtxn->get(main, MDBInVal::fromStruct(std::make_pair(12,13)), out))
     cout << "Got: " << out.get<string_view>() << endl;
   else
     cout << "Got nothing!1"<<endl;
   
-  if(!rwtxn.get(main, MDBInVal::fromStruct(std::make_pair(14,15)), out)) {
+  if(!rwtxn->get(main, MDBInVal::fromStruct(std::make_pair(14,15)), out)) {
     auto res = out.get_struct<pair<int,int>>();
     cout << "Got: " << res.first<<", "<<res.second << endl;
   }
   else
     cout << "Got nothing!1"<<endl;
 
-  rwtxn.put(main, 12.12, 7.3);
+  rwtxn->put(main, 12.12, 7.3);
-  if(!rwtxn.get(main, 12.12, out)) {
+  if(!rwtxn->get(main, 12.12, out)) {
     cout<<"Got: "<< out.get<double>() <<endl;
   }
   else
     cout << "Got nothing!1"<<endl;
 
   
-  rwtxn.commit();
+  rwtxn->commit();
   return 0;
   
   if(argc==1) {
     for(;;) {
       auto rotxn = env->getROTransaction();
       MDBOutVal data;
-      if(!rotxn.get(main, key, data))  {
+      if(!rotxn->get(main, key, data))  {
         cout<<"Counter is "<<data.get<unsigned int>() << endl;
         cout <<data.get<string>() << endl;
         cout<<data.get<string_view>() << endl;
@@ -73,10 +73,10 @@ int main(int argc, char** argv)
         cout<<"Did resize"<<endl;
       }
       auto txn = env->getRWTransaction();
-      txn.put(main, key, MDBInVal(n));
+      txn->put(main, key, MDBInVal(n));
       for(int k=0; k < 100; ++k)
-        txn.put(main, MDBInVal(n+1000*k), MDBInVal(n+1000*k));
+        txn->put(main, MDBInVal(n+1000*k), MDBInVal(n+1000*k));
-      txn.commit();
+      txn->commit();
     }
   }
 }

scale-example.cc

@@ -28,7 +28,7 @@ int main(int argc, char** argv)
     limit = atoi(argv[1]);
   
   cout<<"Counting records.. "; cout.flush();
-  auto cursor=txn.getCursor(dbi);
+  auto cursor = txn->getCursor(dbi);
   MDBOutVal key, data;
   int count=0;
   while(!cursor.get(key, data, count ? MDB_NEXT : MDB_FIRST)) {
@@ -40,15 +40,15 @@ int main(int argc, char** argv)
   cout<<"Have "<<count<<"!"<<endl;
   
   cout<<"Clearing records.. "; cout.flush();
-  mdb_drop(txn, dbi, 0); // clear records
+  mdb_drop(*txn, dbi, 0); // clear records
   cout<<"Done!"<<endl;
 
   cout << "Adding "<<limit<<" values  .. "; cout.flush();
   for(unsigned long n = 0 ; n < limit; ++n) {
-    txn.put(dbi, n, n, MDB_APPEND);
+    txn->put(dbi, n, n, MDB_APPEND);
   }
   cout <<"Done!"<<endl;
   cout <<"Calling commit.. "; cout.flush();
-  txn.commit();
+  txn->commit();
   cout<<"Done!"<<endl;
 }

test-basic.cc

@@ -17,16 +17,16 @@ TEST_CASE("Most basic tests", "[mostbasic]") {
   auto txn = env.getRWTransaction();
   MDBOutVal out;
 
-  REQUIRE(txn.get(main, "lmdb", out) == MDB_NOTFOUND);
+  REQUIRE(txn->get(main, "lmdb", out) == MDB_NOTFOUND);
 
-  txn.put(main, "lmdb", "hot");
+  txn->put(main, "lmdb", "hot");
 
-  REQUIRE(txn.get(main, "lmdb", out) == 0);
+  REQUIRE(txn->get(main, "lmdb", out) == 0);
   REQUIRE(out.get<std::string>() == "hot");
-  txn.abort();
+  txn->abort();
 
   auto rotxn = env.getROTransaction();
-  REQUIRE(rotxn.get(main, "lmdb", out) == MDB_NOTFOUND);
+  REQUIRE(rotxn->get(main, "lmdb", out) == MDB_NOTFOUND);
 }
 
 TEST_CASE("Range tests", "[range]") {
@@ -40,16 +40,16 @@ TEST_CASE("Range tests", "[range]") {
   auto txn = env.getRWTransaction();
   MDBOutVal out;
 
-  REQUIRE(txn.get(main, "lmdb", out) == MDB_NOTFOUND);
+  REQUIRE(txn->get(main, "lmdb", out) == MDB_NOTFOUND);
 
-  txn.put(main, "bert", "hubert");
+  txn->put(main, "bert", "hubert");
-  txn.put(main, "bertt", "1975");
+  txn->put(main, "bertt", "1975");
-  txn.put(main, "berthubert", "lmdb");
+  txn->put(main, "berthubert", "lmdb");
-  txn.put(main, "bert1", "one");
+  txn->put(main, "bert1", "one");
-  txn.put(main, "beru", "not");
+  txn->put(main, "beru", "not");
 
   {
-    auto cursor = txn.getCursor(main);
+    auto cursor = txn->getCursor(main);
     MDBInVal bert("bert");
     MDBOutVal key, val;
     REQUIRE(cursor.lower_bound(bert, key, val) == 0);
@@ -66,12 +66,12 @@ TEST_CASE("Range tests", "[range]") {
 
     REQUIRE(cursor.lower_bound("kees", key, val) == MDB_NOTFOUND);
 
-    txn.commit();
+    txn->commit();
   }
 
   auto rotxn = env.getROTransaction();
   {
-    auto cursor = rotxn.getCursor(main);
+    auto cursor = rotxn->getCursor(main);
     MDBInVal bert("bert");
     MDBOutVal key, val;
     REQUIRE(cursor.lower_bound(bert, key, val) == 0);
@@ -103,17 +103,47 @@ TEST_CASE("moving transactions")
   auto txn = env.getRWTransaction();
   MDBOutVal out;
 
-  REQUIRE(txn.get(main, "lmdb", out) == MDB_NOTFOUND);
+  REQUIRE(txn->get(main, "lmdb", out) == MDB_NOTFOUND);
 
-  txn.put(main, "bert", "hubert");
+  txn->put(main, "bert", "hubert");
-  txn.put(main, "bertt", "1975");
+  txn->put(main, "bertt", "1975");
-  txn.put(main, "berthubert", "lmdb");
+  txn->put(main, "berthubert", "lmdb");
-  txn.put(main, "bert1", "one");
+  txn->put(main, "bert1", "one");
-  txn.put(main, "beru", "not");
+  txn->put(main, "beru", "not");
 
-  auto cursor = txn.getCursor(main);
+  auto cursor = txn->getCursor(main);
   auto txn2 = std::move(txn);
   {
     auto cursor2 = std::move(cursor);
   }
 }
+
+TEST_CASE("transaction inheritance and moving")
+{
+  unlink("./tests");
+
+  MDBEnv env("./tests", MDB_NOSUBDIR, 0600);
+  MDBDbi main = env.openDB("", MDB_CREATE);
+
+  MDBRWCursor cursor;
+  {
+    MDBRWTransaction txn = env.getRWTransaction();
+    MDBOutVal out;
+
+    REQUIRE(txn->get(main, "lmdb", out) == MDB_NOTFOUND);
+
+    // lets just keep this cursor to ensure that it invalidates
+    cursor = txn->getRWCursor(main);
+    txn->put(main, "bert", "hubert");
+    txn->put(main, "bertt", "1975");
+    txn->put(main, "berthubert", "lmdb");
+    txn->put(main, "bert1", "one");
+    txn->put(main, "beru", "not");
+
+    MDBROTransaction ro_txn(std::move(txn));
+    // despite being moved to an ro_txn (which normally commits instead of
+    // aborting by default)
+  }
+
+  CHECK(!cursor);
+}