From 3c57c6a1138a45ad5ea827a6e841af2a011a3b71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Sat, 26 Oct 2019 11:42:38 +0200 Subject: [PATCH] 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 | 24 +++++------ basic-example.cc | 14 +++---- lmdb-safe.cc | 70 +++++++++++++++----------------- lmdb-safe.hh | 100 +++++++++++++++++++--------------------------- lmdb-typed.cc | 2 +- lmdb-typed.hh | 40 +++++++++---------- lmdb-various.cc | 14 +++---- lmdb-view.cc | 6 +-- multi-example.cc | 20 +++++----- rel-example.cc | 14 +++---- resize-example.cc | 26 ++++++------ scale-example.cc | 8 ++-- test-basic.cc | 72 +++++++++++++++++++++++---------- 13 files changed, 209 insertions(+), 201 deletions(-) diff --git a/README.md b/README.md index 2ab0d54..be157cd 100644 --- a/README.md +++ b/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 <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.commit(); + txn->put(dbi, id, score); + 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() << "\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(); ``` @@ -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!"<put(dbi, n, n); } cout <<"Done!"<commit(); cout<<"Done!"<getROTransaction(); MDBOutVal data; - if(!rotxn.get(dbi, "lmdb", data)) { + if(!rotxn->get(dbi, "lmdb", data)) { cout<< "Outside RW transaction, found that lmdb = " << data.get() <openDB("example", MDB_CREATE); auto txn = env->getRWTransaction(); - mdb_drop(txn, dbi, 0); - txn.put(dbi, "lmdb", "great"); + mdb_drop(*txn, dbi, 0); + 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() <commit(); cout<<"Committed data"<getRWTransaction(); - mdb_drop(txn, dbi, 0); - txn.commit(); + mdb_drop(*txn, dbi, 0); + txn->commit(); } diff --git a/lmdb-safe.cc b/lmdb-safe.cc index e0d5687..8ce7c96 100644 --- a/lmdb-safe.cc +++ b/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); - rwt.commit(); + MDBDbi ret = rwt->openDB(dbname, flags); + 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): - MDBROTransaction(parent, openRWTransaction(parent, nullptr, flags)), - d_rw_cursors(new decltype(d_rw_cursors)::element_type()) +MDBRWTransactionImpl::MDBRWTransactionImpl(MDBEnv* parent, int flags): + MDBROTransactionImpl(parent, openRWTransaction(parent, nullptr, flags)), + 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 buf; - std::swap(*d_cursors, buf); + std::swap(d_cursors, buf); for (auto &cursor: buf) { cursor->close(); } } -MDBROTransaction::MDBROTransaction(MDBEnv *parent, int flags): - MDBROTransaction(parent, openROTransaction(parent, nullptr, flags)) +MDBROTransactionImpl::MDBROTransactionImpl(MDBEnv *parent, int 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) { - return; - } - decltype(d_rw_cursors)::element_type buf; - std::swap(*d_rw_cursors, buf); + decltype(d_rw_cursors) 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); } diff --git a/lmdb-safe.hh b/lmdb-safe.hh index a93931f..98814e7 100644 --- a/lmdb-safe.hh +++ b/lmdb-safe.hh @@ -59,8 +59,11 @@ public: MDB_dbi d_dbi; }; -class MDBRWTransaction; -class MDBROTransaction; +class MDBRWTransactionImpl; +class MDBROTransactionImpl; + +using MDBROTransaction = std::unique_ptr; +using MDBRWTransaction = std::unique_ptr; 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> d_cursors; + std::vector 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; - MDBROTransaction &operator=(const MDBROTransaction& src) = delete; + MDBROTransactionImpl(const MDBROTransactionImpl& src) = delete; + MDBROTransactionImpl &operator=(const MDBROTransactionImpl& src) = delete; - MDBROTransaction(MDBROTransaction&& rhs) noexcept: - d_parent(rhs.d_parent), - d_cursors(std::move(rhs.d_cursors)), - d_txn(rhs.d_txn) - { - rhs.d_parent = nullptr; - rhs.d_txn = nullptr; - } + // The move constructor/operator cannot be made safe due to Object Slicing with MDBRWTransaction. + MDBROTransactionImpl(MDBROTransactionImpl&& rhs) = delete; + MDBROTransactionImpl &operator=(MDBROTransactionImpl &&rhs) = delete; - MDBROTransaction &operator=(MDBROTransaction &&rhs) noexcept - { - 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::value>::type> - MDBROTransaction(T&& rhs) = delete; - - virtual ~MDBROTransaction(); + virtual ~MDBROTransactionImpl(); 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 ®istry, MDB_cursor *cursor): d_registry(®istry), 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 +class MDBROCursor : public MDBGenCursor { public: - using MDBGenCursor::MDBGenCursor; + MDBROCursor() = default; + using MDBGenCursor::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> d_rw_cursors; + std::vector 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: - MDBROTransaction(std::move(static_cast(rhs))), - d_rw_cursors(std::move(rhs.d_rw_cursors)) - { + MDBRWTransactionImpl(const MDBRWTransactionImpl& rhs) = delete; + MDBRWTransactionImpl(MDBRWTransactionImpl&& rhs) = delete; + MDBRWTransactionImpl &operator=(const MDBRWTransactionImpl& rhs) = delete; + MDBRWTransactionImpl &operator=(MDBRWTransactionImpl&& rhs) = delete; - } - - MDBRWTransaction &operator=(MDBRWTransaction&& rhs) noexcept - { - MDBROTransaction::operator=(std::move(static_cast(rhs))); - d_rw_cursors = std::move(rhs.d_rw_cursors); - return *this; - } - - ~MDBRWTransaction() override; + ~MDBRWTransactionImpl() 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 +class MDBRWCursor : public MDBGenCursor { public: - using MDBGenCursor::MDBGenCursor; - MDBRWCursor(const MDBRWCursor &src) = default; + MDBRWCursor() = default; + using MDBGenCursor::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; diff --git a/lmdb-typed.cc b/lmdb-typed.cc index ecd630a..b4ecd38 100644 --- a/lmdb-typed.cc +++ b/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)) { diff --git a/lmdb-typed.hh b/lmdb-typed.hh index 7c1179f..b573841 100644 --- a/lmdb-typed.hh +++ b/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(d_parent.d_parent->d_tuple).d_idx, &stat); + mdb_stat(**d_parent.d_txn, std::get(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(), t); @@ -234,7 +234,7 @@ public: uint32_t get(const typename std::tuple_element::type::type& key, T& out) { MDBOutVal id; - if(!d_parent.d_txn->get(std::get(d_parent.d_parent->d_tuple).d_idx, keyConv(key), id)) { + if(!(*d_parent.d_txn)->get(std::get(d_parent.d_parent->d_tuple).d_idx, keyConv(key), id)) { if(get(id.get(), out)) return id.get(); } @@ -245,7 +245,7 @@ public: template uint32_t cardinality() { - auto cursor = d_parent.d_txn->getCursor(std::get(d_parent.d_parent->d_tuple).d_idx); + auto cursor = (*d_parent.d_txn)->getCursor(std::get(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(), 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(), 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 iter_t genbegin(MDB_cursor_op op) { - typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(std::get(d_parent.d_parent->d_tuple).d_idx); + typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get(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 iter_t genfind(const typename std::tuple_element::type::type& key, MDB_cursor_op op) { - typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(std::get(d_parent.d_parent->d_tuple).d_idx); + typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get(d_parent.d_parent->d_tuple).d_idx); std::string keystr = keyConv(key); MDBInVal in(keystr); @@ -498,7 +498,7 @@ public: template std::pair equal_range(const typename std::tuple_element::type::type& key) { - typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(std::get(d_parent.d_parent->d_tuple).d_idx); + typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get(d_parent.d_parent->d_tuple).d_idx); std::string keyString=keyConv(key); MDBInVal in(keyString); @@ -517,7 +517,7 @@ public: template std::pair prefix_range(const typename std::tuple_element::type::type& key) { - typename Parent::cursor_t cursor = d_parent.d_txn->getCursor(std::get(d_parent.d_parent->d_tuple).d_idx); + typename Parent::cursor_t cursor = (*d_parent.d_txn)->getCursor(std::get(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(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; diff --git a/lmdb-various.cc b/lmdb-various.cc index 4ffa457..7377e33 100644 --- a/lmdb-various.cc +++ b/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.commit(); + txn->put(dbi, val, val); + txn->commit(); cout << "Done with transaction "<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"<get(dbi, mv, res)) ++count; } cout<openDB(dbname, 0); + 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)) { diff --git a/multi-example.cc b/multi-example.cc index d61895d..540536e 100644 --- a/multi-example.cc +++ b/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, "lmdb", "hot"); - txn.put(dbi, "lmdb", "fast"); - txn.put(dbi, "lmdb", "zooms"); - txn.put(dbi, "lmdb", "c"); - txn.put(dbi, "mdb", "old name"); + txn->put(dbi, "bdb", "old"); + txn->put(dbi, "lmdb", "hot"); + txn->put(dbi, "lmdb", "fast"); + txn->put(dbi, "lmdb", "zooms"); + txn->put(dbi, "lmdb", "c"); + txn->put(dbi, "mdb", "old name"); string_view v1; - if(!txn.get(dbi, "mdb", v1)) { + if(!txn->get(dbi, "mdb", v1)) { cout<commit(); txn = env->getRWTransaction(); - auto cursor = txn.getRWCursor(dbi); + auto cursor = txn->getRWCursor(dbi); MDBOutVal key, data; diff --git a/rel-example.cc b/rel-example.cc index c38e7b0..9754a31 100644 --- a/rel-example.cc +++ b/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(domainidx, r.domain_id, r.id); - txn.put(nameidx, r.name, r.id); + txn->put(records, r.id, oss.str(), MDB_APPEND); + txn->put(domainidx, r.domain_id, 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="<get(records, data, record)) { Record test; stringstream istr{record.get()}; boost::archive::binary_iarchive oi(istr,boost::archive::no_header ); diff --git a/resize-example.cc b/resize-example.cc index 6904d92..fbd9993 100644 --- a/resize-example.cc +++ b/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, MDBInVal::fromStruct(std::make_pair(12,13)), "hoi dan 12,13"); + 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(14,15)), - MDBInVal::fromStruct(std::make_pair(20,23))); + 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() << endl; else cout << "Got nothing!1"<get(main, MDBInVal::fromStruct(std::make_pair(14,15)), out)) { auto res = out.get_struct>(); cout << "Got: " << res.first<<", "<put(main, 12.12, 7.3); + if(!rwtxn->get(main, 12.12, out)) { cout<<"Got: "<< out.get() <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 "<() << endl; cout <() << endl; cout<() << endl; @@ -73,10 +73,10 @@ int main(int argc, char** argv) cout<<"Did resize"<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.commit(); + txn->put(main, MDBInVal(n+1000*k), MDBInVal(n+1000*k)); + txn->commit(); } } } diff --git a/scale-example.cc b/scale-example.cc index 0ee9065..1efaa36 100644 --- a/scale-example.cc +++ b/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 "<put(dbi, n, n, MDB_APPEND); } cout <<"Done!"<commit(); cout<<"Done!"<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() == "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, "bertt", "1975"); - txn.put(main, "berthubert", "lmdb"); - txn.put(main, "bert1", "one"); - txn.put(main, "beru", "not"); + 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"); { - 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, "bertt", "1975"); - txn.put(main, "berthubert", "lmdb"); - txn.put(main, "bert1", "one"); - txn.put(main, "beru", "not"); + 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"); - 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); +}