From 0f5778f7f0738f7c20ad6be7b70d345b2a7422d1 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Mon, 1 Sep 2025 07:46:26 -0300 Subject: [PATCH 1/5] LockManager AST test --- src/lock/tests/LockManagerTest.cpp | 123 +++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/src/lock/tests/LockManagerTest.cpp b/src/lock/tests/LockManagerTest.cpp index 4ac2ee5ff66..691c0bccdcf 100644 --- a/src/lock/tests/LockManagerTest.cpp +++ b/src/lock/tests/LockManagerTest.cpp @@ -3,6 +3,8 @@ #include #include #include +#include +#include #include #include #include "../common/status.h" @@ -173,6 +175,127 @@ BOOST_AUTO_TEST_CASE(LockUnlockNoWaitTest) } +BOOST_AUTO_TEST_CASE(LockUnlockAstTest) +{ + struct Lock; + + struct ThreadData + { + LockManager* lockManager = nullptr; + std::mutex* globalMutex = nullptr; + std::mutex localMutex; + std::unordered_map locks; + }; + + struct Lock + { + ThreadData* threadData = nullptr; + unsigned key = 0; + SLONG lockId = 0; + }; + + constexpr unsigned THREAD_COUNT = 8u; + constexpr unsigned ITERATION_COUNT = 10'000u; + + ConfigFile configFile(ConfigFile::USE_TEXT, "\n"); + Config config(configFile); + + LockManagerTestCallbacks callbacks; + const string lockManagerId(getUniqueId().c_str()); + auto lockManager = std::make_unique(lockManagerId, &config); + + std::atomic_uint lockSuccess = 0u; + std::atomic_uint lockFail = 0u; + + std::vector threads; + std::latch latch(THREAD_COUNT); + + static const auto ast = [](void* astArg) -> int { + const auto lock = static_cast(astArg); + const auto threadData = lock->threadData; + + std::lock_guard localMutexGuard(threadData->localMutex); + + fb_assert(lock->lockId); + + if (!threadData->lockManager->dequeue(lock->lockId)) + fb_assert(false); + + threadData->locks.erase(lock->lockId); + delete lock; + + return 0; + }; + + std::mutex globalMutex; + + for (unsigned threadNum = 0u; threadNum < THREAD_COUNT; ++threadNum) + { + threads.emplace_back([&, threadNum]() { + ThreadData threadData; + threadData.lockManager = lockManager.get(); + threadData.globalMutex = &globalMutex; + + FbLocalStatus statusVector; + LOCK_OWNER_T ownerId = threadNum + 1; + SLONG ownerHandle = 0; + + lockManager->initializeOwner(&statusVector, ownerId, LCK_OWNER_attachment, &ownerHandle); + + latch.arrive_and_wait(); + + for (unsigned i = 0; i < ITERATION_COUNT; ++i) + { + std::lock_guard globalMutexGuard(globalMutex); + std::lock_guard localMutexGuard(threadData.localMutex); + + const auto lock = new Lock(); + lock->threadData = &threadData; + lock->key = i; + lock->lockId = lockManager->enqueue(callbacks, &statusVector, 0, + LCK_expression, (const UCHAR*) &lock->key, sizeof(lock->key), LCK_EX, + ast, lock, 0, LCK_WAIT, ownerHandle); + + if (lock->lockId) + { + threadData.locks.insert({ lock->lockId, lock }); + ++lockSuccess; + } + else + { + fb_assert(false); + delete lock; + ++lockFail; + } + } + + { // scope + std::lock_guard globalMutexGuard(globalMutex); + std::lock_guard localMutexGuard(threadData.localMutex); + + for (const auto [lockId, lock] : threadData.locks) + { + lockManager->dequeue(lockId); + delete lock; + } + + threadData.locks.clear(); + } + + lockManager->shutdownOwner(callbacks, &ownerHandle); + }); + } + + for (auto& thread : threads) + thread.join(); + + BOOST_CHECK_EQUAL(lockFail.load(), 0u); + BOOST_CHECK_EQUAL(lockSuccess.load(), THREAD_COUNT * ITERATION_COUNT); + + lockManager.reset(); +} + + BOOST_AUTO_TEST_SUITE_END() // LockManagerTests BOOST_AUTO_TEST_SUITE_END() // LockManagerSuite BOOST_AUTO_TEST_SUITE_END() // EngineSuite From 2d5a242acff38f5f4a6afb923e2d987e78f43237 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Mon, 1 Sep 2025 18:01:14 -0300 Subject: [PATCH 2/5] Checkout mutex in LockManagerTestCallbacks --- src/lock/tests/LockManagerTest.cpp | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/lock/tests/LockManagerTest.cpp b/src/lock/tests/LockManagerTest.cpp index 691c0bccdcf..d385f732d7a 100644 --- a/src/lock/tests/LockManagerTest.cpp +++ b/src/lock/tests/LockManagerTest.cpp @@ -21,6 +21,12 @@ namespace { class LockManagerTestCallbacks : public LockManager::Callbacks { + public: + LockManagerTestCallbacks(std::mutex* aMutex = nullptr) + : mutex(aMutex) + { + } + public: ISC_STATUS getCancelState() const { @@ -34,8 +40,18 @@ namespace void checkoutRun(std::function func) const { - func(); + if (mutex) + { + Cleanup lockCleanup([&]() { mutex->lock(); }); + mutex->unlock(); + func(); + } + else + func(); } + + private: + std::mutex* mutex; }; } @@ -182,7 +198,6 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) struct ThreadData { LockManager* lockManager = nullptr; - std::mutex* globalMutex = nullptr; std::mutex localMutex; std::unordered_map locks; }; @@ -200,7 +215,6 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) ConfigFile configFile(ConfigFile::USE_TEXT, "\n"); Config config(configFile); - LockManagerTestCallbacks callbacks; const string lockManagerId(getUniqueId().c_str()); auto lockManager = std::make_unique(lockManagerId, &config); @@ -234,8 +248,8 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) threads.emplace_back([&, threadNum]() { ThreadData threadData; threadData.lockManager = lockManager.get(); - threadData.globalMutex = &globalMutex; + LockManagerTestCallbacks callbacks(&threadData.localMutex); FbLocalStatus statusVector; LOCK_OWNER_T ownerId = threadNum + 1; SLONG ownerHandle = 0; From 968e1522157376dd51b5c2fa2c0c0e4bb875631e Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Tue, 2 Sep 2025 21:09:42 -0300 Subject: [PATCH 3/5] Remove test globalMutex - thanks Vlad for the patch --- src/lock/tests/LockManagerTest.cpp | 59 ++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/src/lock/tests/LockManagerTest.cpp b/src/lock/tests/LockManagerTest.cpp index d385f732d7a..738708e2a16 100644 --- a/src/lock/tests/LockManagerTest.cpp +++ b/src/lock/tests/LockManagerTest.cpp @@ -197,9 +197,12 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) struct ThreadData { + std::thread::id threadId; LockManager* lockManager = nullptr; std::mutex localMutex; std::unordered_map locks; + SLONG ownerHandle; + bool shutdown = false; }; struct Lock @@ -207,10 +210,12 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) ThreadData* threadData = nullptr; unsigned key = 0; SLONG lockId = 0; + bool locked = false; + bool blocking = false; }; constexpr unsigned THREAD_COUNT = 8u; - constexpr unsigned ITERATION_COUNT = 10'000u; + constexpr unsigned ITERATION_COUNT = 100'000u; ConfigFile configFile(ConfigFile::USE_TEXT, "\n"); Config config(configFile); @@ -230,23 +235,33 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) std::lock_guard localMutexGuard(threadData->localMutex); - fb_assert(lock->lockId); + if (threadData->shutdown) + return 0; - if (!threadData->lockManager->dequeue(lock->lockId)) - fb_assert(false); + fb_assert(!lock->locked || lock->lockId); - threadData->locks.erase(lock->lockId); - delete lock; + if (lock->locked) + { + if (!threadData->lockManager->dequeue(lock->lockId)) + fb_assert(false); + + const auto num = threadData->locks.erase(lock->lockId); + fb_assert(num == 1); + delete lock; + } + else + lock->blocking = true; return 0; }; - std::mutex globalMutex; + //std::mutex globalMutex; for (unsigned threadNum = 0u; threadNum < THREAD_COUNT; ++threadNum) { threads.emplace_back([&, threadNum]() { ThreadData threadData; + threadData.threadId = std::this_thread::get_id(); threadData.lockManager = lockManager.get(); LockManagerTestCallbacks callbacks(&threadData.localMutex); @@ -255,25 +270,37 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) SLONG ownerHandle = 0; lockManager->initializeOwner(&statusVector, ownerId, LCK_OWNER_attachment, &ownerHandle); + threadData.ownerHandle = ownerHandle; latch.arrive_and_wait(); for (unsigned i = 0; i < ITERATION_COUNT; ++i) { - std::lock_guard globalMutexGuard(globalMutex); + //std::lock_guard globalMutexGuard(globalMutex); std::lock_guard localMutexGuard(threadData.localMutex); const auto lock = new Lock(); lock->threadData = &threadData; lock->key = i; + lock->lockId = lockManager->enqueue(callbacks, &statusVector, 0, LCK_expression, (const UCHAR*) &lock->key, sizeof(lock->key), LCK_EX, ast, lock, 0, LCK_WAIT, ownerHandle); if (lock->lockId) { - threadData.locks.insert({ lock->lockId, lock }); ++lockSuccess; + + if (lock->blocking) + { + lockManager->dequeue(lock->lockId); + delete lock; + } + else + { + lock->locked = true; + threadData.locks.insert({ lock->lockId, lock }); + } } else { @@ -284,19 +311,27 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) } { // scope - std::lock_guard globalMutexGuard(globalMutex); + //std::lock_guard globalMutexGuard(globalMutex); std::lock_guard localMutexGuard(threadData.localMutex); + threadData.shutdown = true; + for (const auto [lockId, lock] : threadData.locks) { + fb_assert(!lock->blocking); + fb_assert(lock->locked); + fb_assert(lock->lockId); lockManager->dequeue(lockId); - delete lock; } + lockManager->shutdownOwner(callbacks, &ownerHandle); + + for (const auto [lockId, lock] : threadData.locks) + delete lock; + threadData.locks.clear(); } - lockManager->shutdownOwner(callbacks, &ownerHandle); }); } From a146df9af521a290d8b2bde61313fd31ce05fc72 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Tue, 2 Sep 2025 21:19:15 -0300 Subject: [PATCH 4/5] Misc --- src/lock/tests/LockManagerTest.cpp | 64 ++++++++++++------------------ 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/src/lock/tests/LockManagerTest.cpp b/src/lock/tests/LockManagerTest.cpp index 738708e2a16..592eb75d2d7 100644 --- a/src/lock/tests/LockManagerTest.cpp +++ b/src/lock/tests/LockManagerTest.cpp @@ -1,9 +1,13 @@ #include "firebird.h" #include "boost/test/unit_test.hpp" #include +#include +#include #include #include +#include #include +#include #include #include #include @@ -200,8 +204,8 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) std::thread::id threadId; LockManager* lockManager = nullptr; std::mutex localMutex; - std::unordered_map locks; - SLONG ownerHandle; + std::unordered_map> locks; + SLONG ownerHandle = 0; bool shutdown = false; }; @@ -209,13 +213,12 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) { ThreadData* threadData = nullptr; unsigned key = 0; - SLONG lockId = 0; - bool locked = false; + std::optional lockId; bool blocking = false; }; constexpr unsigned THREAD_COUNT = 8u; - constexpr unsigned ITERATION_COUNT = 100'000u; + constexpr unsigned ITERATION_COUNT = 10'000u; ConfigFile configFile(ConfigFile::USE_TEXT, "\n"); Config config(configFile); @@ -238,16 +241,15 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) if (threadData->shutdown) return 0; - fb_assert(!lock->locked || lock->lockId); + fb_assert(!lock->lockId.has_value() || lock->lockId.value() != 0); - if (lock->locked) + if (lock->lockId.has_value()) { - if (!threadData->lockManager->dequeue(lock->lockId)) + if (!threadData->lockManager->dequeue(lock->lockId.value())) fb_assert(false); - const auto num = threadData->locks.erase(lock->lockId); - fb_assert(num == 1); - delete lock; + [[maybe_unused]] const auto erasedCount = threadData->locks.erase(lock->lockId.value()); + fb_assert(erasedCount == 1); } else lock->blocking = true; @@ -255,8 +257,6 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) return 0; }; - //std::mutex globalMutex; - for (unsigned threadNum = 0u; threadNum < THREAD_COUNT; ++threadNum) { threads.emplace_back([&, threadNum]() { @@ -267,67 +267,55 @@ BOOST_AUTO_TEST_CASE(LockUnlockAstTest) LockManagerTestCallbacks callbacks(&threadData.localMutex); FbLocalStatus statusVector; LOCK_OWNER_T ownerId = threadNum + 1; - SLONG ownerHandle = 0; - lockManager->initializeOwner(&statusVector, ownerId, LCK_OWNER_attachment, &ownerHandle); - threadData.ownerHandle = ownerHandle; + lockManager->initializeOwner(&statusVector, ownerId, LCK_OWNER_attachment, &threadData.ownerHandle); latch.arrive_and_wait(); for (unsigned i = 0; i < ITERATION_COUNT; ++i) { - //std::lock_guard globalMutexGuard(globalMutex); - std::lock_guard localMutexGuard(threadData.localMutex); - - const auto lock = new Lock(); + auto lock = std::make_unique(); lock->threadData = &threadData; lock->key = i; - lock->lockId = lockManager->enqueue(callbacks, &statusVector, 0, + std::lock_guard localMutexGuard(threadData.localMutex); + + const auto lockId = lockManager->enqueue(callbacks, &statusVector, 0, LCK_expression, (const UCHAR*) &lock->key, sizeof(lock->key), LCK_EX, - ast, lock, 0, LCK_WAIT, ownerHandle); + ast, lock.get(), 0, LCK_WAIT, threadData.ownerHandle); - if (lock->lockId) + if (lockId) { ++lockSuccess; if (lock->blocking) - { - lockManager->dequeue(lock->lockId); - delete lock; - } + lockManager->dequeue(lockId); else { - lock->locked = true; - threadData.locks.insert({ lock->lockId, lock }); + lock->lockId = lockId; + threadData.locks.insert({ lockId, std::move(lock) }); } } else { fb_assert(false); - delete lock; ++lockFail; } } { // scope - //std::lock_guard globalMutexGuard(globalMutex); std::lock_guard localMutexGuard(threadData.localMutex); threadData.shutdown = true; - for (const auto [lockId, lock] : threadData.locks) + for (const auto& [lockId, lock] : threadData.locks) { fb_assert(!lock->blocking); - fb_assert(lock->locked); - fb_assert(lock->lockId); + fb_assert(lock->lockId.has_value() && lock->lockId.value() == lockId); lockManager->dequeue(lockId); } - lockManager->shutdownOwner(callbacks, &ownerHandle); - - for (const auto [lockId, lock] : threadData.locks) - delete lock; + lockManager->shutdownOwner(callbacks, &threadData.ownerHandle); threadData.locks.clear(); } From d80a997865070ee21ecb3a284dc45929a5163b56 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Wed, 3 Sep 2025 07:25:46 -0300 Subject: [PATCH 5/5] Misc --- src/lock/tests/LockManagerTest.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lock/tests/LockManagerTest.cpp b/src/lock/tests/LockManagerTest.cpp index 592eb75d2d7..f407cd0b2ab 100644 --- a/src/lock/tests/LockManagerTest.cpp +++ b/src/lock/tests/LockManagerTest.cpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include