From ef86d0adf97abc071f71106a187ca02a39a98e50 Mon Sep 17 00:00:00 2001 From: blue Date: Tue, 24 Dec 2024 18:35:56 +0200 Subject: [PATCH] Some reformation, test cases and first bugfixes --- src/cursor.hpp | 50 +++++++++++++----------------------------- src/storage.cpp | 21 +++++++++++++----- src/storage.h | 4 ++-- test/cachecursor.cpp | 35 +++++++++++++++++++++++++++++ test/storagecursor.cpp | 30 +++++++++++++++++++++---- 5 files changed, 93 insertions(+), 47 deletions(-) diff --git a/src/cursor.hpp b/src/cursor.hpp index 79fcff0..875b764 100644 --- a/src/cursor.hpp +++ b/src/cursor.hpp @@ -195,14 +195,12 @@ void LMDBAL::Cursor::terminated () { switch (state) { case openedPublic: storage->_mdbCursorClose(cursor); - state = closed; - break; + break; case openedPrivate: - storage->closeCursorTransaction(cursor); - + storage->closeCursorTransaction(cursor, true); state = closed; - break; + break; default: break; } @@ -228,7 +226,7 @@ void LMDBAL::Cursor::open () { switch (state) { case closed: - storage->openCursorTransaction(&cursor); + storage->openCursorTransaction(&cursor, false); state = openedPrivate; break; default: @@ -297,30 +295,15 @@ void LMDBAL::Cursor::renew () { storage->ensureOpened(renewCursorMethodName); switch (state) { - case openedPrivate: { - TransactionID txn = storage->_mdbCursorTxn(cursor); - storage->abortTransaction(txn); - storage->transactionAborted(txn); - - txn = storage->beginReadOnlyTransaction(); - int result = storage->_mdbCursorRenew(txn, cursor); - if (result != MDB_SUCCESS) - storage->throwUnknown(result, txn); - - storage->transactionStarted(txn, true); - state = openedPrivate; - } - case openedPublic: { + case openedPrivate: + storage->closeCursorTransaction(cursor, false); + storage->openCursorTransaction(&cursor, true); + break; + case openedPublic: storage->disconnectCursorFromTransaction(id, cursor); - TransactionID txn = storage->beginReadOnlyTransaction(); - int result = storage->_mdbCursorRenew(txn, cursor); - if (result != MDB_SUCCESS) - storage->throwUnknown(result, txn); - - storage->transactionStarted(txn, true); - storage->attachCursorToTransaction(id, cursor, this); + storage->openCursorTransaction(&cursor, true); state = openedPrivate; - } break; + break; default: break; } @@ -355,16 +338,14 @@ void LMDBAL::Cursor::renew (const Transaction& transaction) { TransactionID txn = storage->extractTransactionId(transaction, renewCursorMethodName); switch (state) { case openedPrivate: { - TransactionID txnOld = storage->_mdbCursorTxn(cursor); - storage->abortTransaction(txnOld); - storage->transactionAborted(txnOld); - + storage->closeCursorTransaction(cursor, false); int result = storage->_mdbCursorRenew(txn, cursor); if (result != MDB_SUCCESS) storage->throwUnknown(result); + storage->attachCursorToTransaction(id, cursor, this); state = openedPublic; - } + } break; case openedPublic: { storage->disconnectCursorFromTransaction(id, cursor); int result = storage->_mdbCursorRenew(txn, cursor); @@ -372,7 +353,6 @@ void LMDBAL::Cursor::renew (const Transaction& transaction) { storage->throwUnknown(result); storage->attachCursorToTransaction(id, cursor, this); - state = openedPublic; } break; default: break; @@ -399,7 +379,7 @@ void LMDBAL::Cursor::close () { state = closed; break; case openedPrivate: - storage->closeCursorTransaction(cursor); + storage->closeCursorTransaction(cursor, true); state = closed; break; diff --git a/src/storage.cpp b/src/storage.cpp index 4847760..4359987 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -180,16 +180,22 @@ void LMDBAL::iStorage::attachCursorToTransaction (uint32_t cursorId, MDB_cursor* * * This method is ment to be called from LMDBAL::Cursor * - * \param[out] cursor - cursor handle + * \param[out] cursor - cursor handle + * \param[in] renew - true if instead of opening cursor should be renewed * * \exception LMDBAL::Closed thrown if you try to open the cursor on a closed database * \exception LMDBAL::Unknown thrown if there was a problem opening the cursor by the lmdb, or to begin a transaction */ -void LMDBAL::iStorage::openCursorTransaction (MDB_cursor** cursor) const { +void LMDBAL::iStorage::openCursorTransaction (MDB_cursor** cursor, bool renew) const { ensureOpened(openCursorTransactionMethodName); TransactionID txn = beginReadOnlyTransaction(); - int result = _mdbCursorOpen(txn, cursor); + int result; + if (renew) + result = _mdbCursorRenew(txn, *cursor); + else + result = _mdbCursorOpen(txn, cursor); + if (result != MDB_SUCCESS) throwUnknown(result, txn); @@ -201,11 +207,14 @@ void LMDBAL::iStorage::openCursorTransaction (MDB_cursor** cursor) const { * * This method is ment to be called from LMDBAL::Cursor * - * \param[in] cursor - cursor handle + * \param[in] cursor - cursor handle + * \param[in] closeCursor - true if the cursor should also get closed, false if you wish to leave it open */ -void LMDBAL::iStorage::closeCursorTransaction (MDB_cursor* cursor) const { +void LMDBAL::iStorage::closeCursorTransaction (MDB_cursor* cursor, bool closeCursor) const { TransactionID txn = _mdbCursorTxn(cursor); - _mdbCursorClose(cursor); + if (closeCursor) + _mdbCursorClose(cursor); + abortTransaction(txn); transactionAborted(txn); } diff --git a/src/storage.h b/src/storage.h index 08c37ba..99141e0 100644 --- a/src/storage.h +++ b/src/storage.h @@ -76,8 +76,8 @@ protected: virtual int drop(TransactionID transaction); virtual SizeType count(TransactionID txn) const; - void openCursorTransaction(MDB_cursor** cursor) const; - void closeCursorTransaction(MDB_cursor* cursor) const; + void openCursorTransaction(MDB_cursor** cursor, bool renew = false) const; + void closeCursorTransaction(MDB_cursor* cursor, bool closeCursor = false) const; void attachCursorToTransaction(uint32_t cursorId, MDB_cursor* cursorHandle, iCursor* pointer) const; void disconnectCursorFromTransaction(uint32_t cursorId, MDB_cursor* cursorHandle) const; diff --git a/test/cachecursor.cpp b/test/cachecursor.cpp index ec15b46..b965c69 100644 --- a/test/cachecursor.cpp +++ b/test/cachecursor.cpp @@ -436,5 +436,40 @@ TEST_F(CacheCursorTest, CornerCases) { EXPECT_EQ(element.first, reference->first); EXPECT_EQ(element.second, reference->second); EXPECT_THROW(cursor.prev(), LMDBAL::NotFound); + + cursor.close(); } +TEST_F(CacheCursorTest, TerminatedTransaction) { + LMDBAL::Cursor cr = cache->createCursor(); + + { + LMDBAL::Transaction txn = db->beginReadOnlyTransaction(); + cr.open(txn); + EXPECT_NO_THROW(cr.first()); + } + + EXPECT_FALSE(cr.opened()); + + LMDBAL::Transaction txn2; + { + LMDBAL::Transaction txn = db->beginReadOnlyTransaction(); + EXPECT_TRUE(txn.isActive()); + EXPECT_FALSE(txn2.isActive()); + + cr.open(txn); + EXPECT_TRUE(cr.opened()); + + txn2 = std::move(txn); + EXPECT_FALSE(txn.isActive()); + EXPECT_TRUE(txn2.isActive()); + EXPECT_TRUE(cr.opened()); + } + + EXPECT_TRUE(txn2.isActive()); + EXPECT_TRUE(cr.opened()); + + txn2.terminate(); + EXPECT_FALSE(txn2.isActive()); + EXPECT_FALSE(cr.opened()); +} diff --git a/test/storagecursor.cpp b/test/storagecursor.cpp index 7e332fb..92279b1 100644 --- a/test/storagecursor.cpp +++ b/test/storagecursor.cpp @@ -419,13 +419,35 @@ TEST_F(StorageCursorTest, CornerCases) { } TEST_F(StorageCursorTest, TerminatedTransaction) { - LMDBAL::Cursor emptyCursor = table->createCursor(); + LMDBAL::Cursor cr = table->createCursor(); { LMDBAL::Transaction txn = db->beginReadOnlyTransaction(); - cursor.open(txn); - EXPECT_NO_THROW(cursor.first()); + cr.open(txn); + EXPECT_NO_THROW(cr.first()); } - EXPECT_FALSE(cursor.opened()); + EXPECT_FALSE(cr.opened()); + + LMDBAL::Transaction txn2; + { + LMDBAL::Transaction txn = db->beginReadOnlyTransaction(); + EXPECT_TRUE(txn.isActive()); + EXPECT_FALSE(txn2.isActive()); + + cr.open(txn); + EXPECT_TRUE(cr.opened()); + + txn2 = std::move(txn); + EXPECT_FALSE(txn.isActive()); + EXPECT_TRUE(txn2.isActive()); + EXPECT_TRUE(cr.opened()); + } + + EXPECT_TRUE(txn2.isActive()); + EXPECT_TRUE(cr.opened()); + + txn2.terminate(); + EXPECT_FALSE(txn2.isActive()); + EXPECT_FALSE(cr.opened()); }