From 6e2eb2701eb2e99b7847593e7367b068028bb8a3 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Wed, 27 Aug 2025 21:39:50 -0300 Subject: [PATCH 1/9] Decouple LockManager from engine --- src/jrd/lck.cpp | 41 +++++++++++++----- src/jrd/lck.h | 16 +++++++ src/lock/lock.cpp | 97 ++++++++++++++++++++++--------------------- src/lock/lock_proto.h | 36 ++++++++++------ 4 files changed, 119 insertions(+), 71 deletions(-) diff --git a/src/jrd/lck.cpp b/src/jrd/lck.cpp index 8a3baf6bf8d..c2fa9d04c94 100644 --- a/src/jrd/lck.cpp +++ b/src/jrd/lck.cpp @@ -95,6 +95,24 @@ namespace } #endif + +ISC_STATUS LockManagerEngineCallbacks::getCancelState() const +{ + return tdbb->getCancelState(); +} + +ULONG LockManagerEngineCallbacks::adjustWait(ULONG wait) const +{ + return tdbb->adjustWait(wait); +} + +void LockManagerEngineCallbacks::checkoutRun(std::function func) const +{ + EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); + func(); +} + + // globals and macros inline LOCK_OWNER_T LCK_OWNER_ID_DBB(thread_db* tdbb) @@ -155,8 +173,8 @@ inline bool CONVERT(thread_db* tdbb, CheckStatusWrapper* statusVector, Lock* loc return lock->lck_compatible ? internal_enqueue(tdbb, statusVector, lock, level, wait, true) : - dbb->lockManager()->convert(tdbb, statusVector, lock->lck_id, level, wait, lock->lck_ast, - lock->lck_object); + dbb->lockManager()->convert(LockManagerEngineCallbacks(tdbb), statusVector, lock->lck_id, level, + wait, lock->lck_ast, lock->lck_object); } inline void DEQUEUE(thread_db* tdbb, Lock* lock) @@ -177,7 +195,7 @@ inline USHORT DOWNGRADE(thread_db* tdbb, Lock* lock) USHORT ret = lock->lck_compatible ? internal_downgrade(tdbb, &statusVector, lock) : - dbb->lockManager()->downgrade(tdbb, &statusVector, lock->lck_id); + dbb->lockManager()->downgrade(LockManagerEngineCallbacks(tdbb), &statusVector, lock->lck_id); fb_assert(statusVector.isEmpty()); @@ -499,7 +517,7 @@ void LCK_fini(thread_db* tdbb, enum lck_owner_t owner_type) } if (*owner_handle_ptr) - dbb->lockManager()->shutdownOwner(tdbb, owner_handle_ptr); + dbb->lockManager()->shutdownOwner(LockManagerEngineCallbacks(tdbb), owner_handle_ptr); } @@ -841,7 +859,8 @@ void LCK_re_post(thread_db* tdbb, Lock* lock) return; } - dbb->lockManager()->repost(tdbb, lock->lck_ast, lock->lck_object, lock->lck_owner_handle); + dbb->lockManager()->repost(LockManagerEngineCallbacks(tdbb), + lock->lck_ast, lock->lck_object, lock->lck_owner_handle); fb_assert(LCK_CHECK_LOCK(lock)); } @@ -945,7 +964,7 @@ static void enqueue(thread_db* tdbb, CheckStatusWrapper* statusVector, Lock* loc fb_assert(LCK_CHECK_LOCK(lock)); - lock->lck_id = dbb->lockManager()->enqueue(tdbb, statusVector, lock->lck_id, + lock->lck_id = dbb->lockManager()->enqueue(LockManagerEngineCallbacks(tdbb), statusVector, lock->lck_id, lock->lck_type, lock->getKeyPtr(), lock->lck_length, level, lock->lck_ast, lock->lck_object, lock->lck_data, wait, lock->lck_owner_handle); @@ -1335,8 +1354,8 @@ static USHORT internal_downgrade(thread_db* tdbb, CheckStatusWrapper* statusVect if (level < first->lck_physical) { - if (dbb->lockManager()->convert(tdbb, statusVector, first->lck_id, level, LCK_NO_WAIT, - external_ast, first)) + if (dbb->lockManager()->convert(LockManagerEngineCallbacks(tdbb), statusVector, + first->lck_id, level, LCK_NO_WAIT, external_ast, first)) { for (Lock* lock = first; lock; lock = lock->lck_identical) { @@ -1402,8 +1421,8 @@ static bool internal_enqueue(thread_db* tdbb, CheckStatusWrapper* statusVector, if (level > match->lck_physical) { - if (!dbb->lockManager()->convert(tdbb, statusVector, match->lck_id, level, wait, - external_ast, lock)) + if (!dbb->lockManager()->convert(LockManagerEngineCallbacks(tdbb), statusVector, + match->lck_id, level, wait, external_ast, lock)) { return false; } @@ -1431,7 +1450,7 @@ static bool internal_enqueue(thread_db* tdbb, CheckStatusWrapper* statusVector, // enqueue the lock, but swap out the ast and the ast argument // with the local ast handler, passing it the lock block itself - lock->lck_id = dbb->lockManager()->enqueue(tdbb, statusVector, lock->lck_id, + lock->lck_id = dbb->lockManager()->enqueue(LockManagerEngineCallbacks(tdbb), statusVector, lock->lck_id, lock->lck_type, lock->getKeyPtr(), lock->lck_length, level, external_ast, lock, lock->lck_data, wait, lock->lck_owner_handle); diff --git a/src/jrd/lck.h b/src/jrd/lck.h index dc8f9274984..08ea8992f0e 100644 --- a/src/jrd/lck.h +++ b/src/jrd/lck.h @@ -86,6 +86,22 @@ enum lck_owner_t { LCK_OWNER_attachment // An attachment is the owner of the lock }; +class LockManagerEngineCallbacks final : public LockManager::Callbacks +{ +public: + LockManagerEngineCallbacks(thread_db* aTdbb) + : tdbb(aTdbb) + { + } + + ISC_STATUS getCancelState() const override; + ULONG adjustWait(ULONG wait) const override; + void checkoutRun(std::function func) const override; + +private: + thread_db* const tdbb; +}; + class Lock : public pool_alloc_rpt { public: diff --git a/src/lock/lock.cpp b/src/lock/lock.cpp index b04a7dee847..09ee3418a46 100644 --- a/src/lock/lock.cpp +++ b/src/lock/lock.cpp @@ -40,9 +40,8 @@ #include "firebird.h" #include "../lock/lock_proto.h" +#include "../common/StatusHolder.h" #include "../common/ThreadStart.h" -#include "../jrd/jrd.h" -#include "../jrd/Attachment.h" #include "iberror.h" #include "../yvalve/gds_proto.h" #include "../common/gdsassert.h" @@ -381,7 +380,7 @@ bool LockManager::initializeOwner(CheckStatusWrapper* statusVector, } -void LockManager::shutdownOwner(thread_db* tdbb, SRQ_PTR* owner_handle) +void LockManager::shutdownOwner(const Callbacks& callbacks, SRQ_PTR* owner_handle) { /************************************** * @@ -413,8 +412,9 @@ void LockManager::shutdownOwner(thread_db* tdbb, SRQ_PTR* owner_handle) { { // checkout scope LockTableCheckout checkout(this, FB_FUNCTION); - EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - Thread::sleep(10); + callbacks.checkoutRun([] { + Thread::sleep(10); + }); } owner = (own*) SRQ_ABS_PTR(owner_offset); @@ -431,7 +431,7 @@ void LockManager::shutdownOwner(thread_db* tdbb, SRQ_PTR* owner_handle) } -SRQ_PTR LockManager::enqueue(thread_db* tdbb, +SRQ_PTR LockManager::enqueue(const Callbacks& callbacks, CheckStatusWrapper* statusVector, SRQ_PTR prior_request, const USHORT series, @@ -528,7 +528,7 @@ SRQ_PTR LockManager::enqueue(thread_db* tdbb, insert_tail(&lock->lbl_requests, &request->lrq_lbl_requests); request->lrq_data = data; - if (grant_or_que(tdbb, request, lock, lck_wait)) + if (grant_or_que(callbacks, request, lock, lck_wait)) return request_offset; Arg::Gds(lck_wait > 0 ? isc_deadlock : lck_wait < 0 ? isc_lock_timeout : @@ -584,7 +584,7 @@ SRQ_PTR LockManager::enqueue(thread_db* tdbb, } -bool LockManager::convert(thread_db* tdbb, +bool LockManager::convert(const Callbacks& callbacks, CheckStatusWrapper* statusVector, SRQ_PTR request_offset, UCHAR type, @@ -623,14 +623,14 @@ bool LockManager::convert(thread_db* tdbb, ++(m_sharedMemory->getHeader()->lhb_operations[0]); const bool result = - internal_convert(tdbb, statusVector, request_offset, type, lck_wait, + internal_convert(callbacks, statusVector, request_offset, type, lck_wait, ast_routine, ast_argument); return result; } -UCHAR LockManager::downgrade(thread_db* tdbb, +UCHAR LockManager::downgrade(const Callbacks& callbacks, CheckStatusWrapper* statusVector, const SRQ_PTR request_offset) { @@ -688,7 +688,7 @@ UCHAR LockManager::downgrade(thread_db* tdbb, } else { - internal_convert(tdbb, statusVector, request_offset, state, LCK_NO_WAIT, + internal_convert(callbacks, statusVector, request_offset, state, LCK_NO_WAIT, request->lrq_ast_routine, request->lrq_ast_argument); } @@ -733,7 +733,7 @@ bool LockManager::dequeue(const SRQ_PTR request_offset) } -void LockManager::repost(thread_db* tdbb, lock_ast_t ast, void* arg, SRQ_PTR owner_offset) +void LockManager::repost(const Callbacks& callbacks, lock_ast_t ast, void* arg, SRQ_PTR owner_offset) { /************************************** * @@ -790,7 +790,7 @@ void LockManager::repost(thread_db* tdbb, lock_ast_t ast, void* arg, SRQ_PTR own DEBUG_DELAY; if (!(owner->own_flags & OWN_signaled)) - signal_owner(tdbb, owner); + signal_owner(callbacks, owner); } @@ -1340,7 +1340,8 @@ lbl* LockManager::alloc_lock(USHORT length, CheckStatusWrapper* statusVector) } -void LockManager::blocking_action(thread_db* tdbb, SRQ_PTR blocking_owner_offset) +void LockManager::blocking_action(const Callbacks* callbacks, + SRQ_PTR blocking_owner_offset) { /************************************** * @@ -1397,20 +1398,22 @@ void LockManager::blocking_action(thread_db* tdbb, SRQ_PTR blocking_owner_offset { // checkout scope LockTableCheckout checkout(this, FB_FUNCTION); - EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - try - { - (*routine)(arg); - } - catch (const Exception& ex) - { - iscLogException("Exception from AST routine - this should never happen", ex); - } - catch (...) - { - gds__log("Unknown C++ exception from AST routine - this should never happen"); - } + fb_assert(callbacks); + callbacks->checkoutRun([&] { + try + { + (*routine)(arg); + } + catch (const Exception& ex) + { + iscLogException("Exception from AST routine - this should never happen", ex); + } + catch (...) + { + gds__log("Unknown C++ exception from AST routine - this should never happen"); + } + }); } owner = (own*) SRQ_ABS_PTR(blocking_owner_offset); @@ -1485,7 +1488,7 @@ void LockManager::blocking_action_thread() { const SRQ_PTR owner_offset = SRQ_REL_PTR(owner); guard.setOwner(owner_offset); - blocking_action(NULL, owner_offset); + blocking_action(nullptr, owner_offset); completed = false; break; } @@ -2163,7 +2166,7 @@ void LockManager::grant(lrq* request, lbl* lock) } -bool LockManager::grant_or_que(thread_db* tdbb, lrq* request, lbl* lock, SSHORT lck_wait) +bool LockManager::grant_or_que(const Callbacks& callbacks, lrq* request, lbl* lock, SSHORT lck_wait) { /************************************** * @@ -2200,7 +2203,7 @@ bool LockManager::grant_or_que(thread_db* tdbb, lrq* request, lbl* lock, SSHORT { const SRQ_PTR request_offset = SRQ_REL_PTR(request); - wait_for_request(tdbb, request, lck_wait); + wait_for_request(callbacks, request, lck_wait); request = (lrq*) SRQ_ABS_PTR(request_offset); @@ -2458,7 +2461,7 @@ void LockManager::insert_tail(SRQ lock_srq, SRQ node) } -bool LockManager::internal_convert(thread_db* tdbb, +bool LockManager::internal_convert(const Callbacks& callbacks, CheckStatusWrapper* statusVector, SRQ_PTR request_offset, UCHAR type, @@ -2520,7 +2523,7 @@ bool LockManager::internal_convert(thread_db* tdbb, else new_ast = false; - wait_for_request(tdbb, request, lck_wait); + wait_for_request(callbacks, request, lck_wait); request = (lrq*) SRQ_ABS_PTR(request_offset); lock = (lbl*) SRQ_ABS_PTR(request->lrq_lock); @@ -2605,7 +2608,7 @@ USHORT LockManager::lock_state(const lbl* lock) } -void LockManager::post_blockage(thread_db* tdbb, lrq* request, lbl* lock) +void LockManager::post_blockage(const Callbacks& callbacks, lrq* request, lbl* lock) { /************************************** * @@ -2673,7 +2676,7 @@ void LockManager::post_blockage(thread_db* tdbb, lrq* request, lbl* lock) if (blocking_owner->own_count && !(blocking_owner->own_flags & OWN_signaled) && - !signal_owner(tdbb, blocking_owner)) + !signal_owner(callbacks, blocking_owner)) { dead_processes.add(blocking_owner->own_process); } @@ -3169,7 +3172,7 @@ void LockManager::release_request(lrq* request) } -bool LockManager::signal_owner(thread_db* tdbb, own* blocking_owner) +bool LockManager::signal_owner(const Callbacks& callbacks, own* blocking_owner) { /************************************** * @@ -3202,7 +3205,7 @@ bool LockManager::signal_owner(thread_db* tdbb, own* blocking_owner) if (process->prc_process_id == PID) { DEBUG_DELAY; - blocking_action(tdbb, SRQ_REL_PTR(blocking_owner)); + blocking_action(&callbacks, SRQ_REL_PTR(blocking_owner)); DEBUG_DELAY; return true; } @@ -3706,7 +3709,7 @@ void LockManager::validate_shb(const SRQ_PTR shb_ptr) } -void LockManager::wait_for_request(thread_db* tdbb, lrq* request, SSHORT lck_wait) +void LockManager::wait_for_request(const Callbacks& callbacks, lrq* request, SSHORT lck_wait) { /************************************** * @@ -3763,7 +3766,7 @@ void LockManager::wait_for_request(thread_db* tdbb, lrq* request, SSHORT lck_wai // Post blockage. If the blocking owner has disappeared, the blockage // may clear spontaneously. - post_blockage(tdbb, request, lock); + post_blockage(callbacks, request, lock); post_history(his_wait, owner_offset, lock_offset, request_offset, true); owner = (own*) SRQ_ABS_PTR(owner_offset); @@ -3776,7 +3779,7 @@ void LockManager::wait_for_request(thread_db* tdbb, lrq* request, SSHORT lck_wai // out the time when the lock request will timeout const time_t lock_timeout = (lck_wait < 0) ? current_time + (-lck_wait) : 0; - time_t deadlock_timeout = current_time + tdbb->adjustWait(scan_interval); + time_t deadlock_timeout = current_time + callbacks.adjustWait(scan_interval); // Wait in a loop until the lock becomes available @@ -3822,9 +3825,10 @@ void LockManager::wait_for_request(thread_db* tdbb, lrq* request, SSHORT lck_wai } { // scope - EngineCheckout cout(tdbb, FB_FUNCTION, EngineCheckout::UNNECESSARY); - ret = m_sharedMemory->eventWait(&owner->own_wakeup, value, (timeout - current_time) * 1000000); - --m_waitingOwners; + callbacks.checkoutRun([&] { + ret = m_sharedMemory->eventWait(&owner->own_wakeup, value, (timeout - current_time) * 1000000); + --m_waitingOwners; + }); } } @@ -3881,8 +3885,7 @@ void LockManager::wait_for_request(thread_db* tdbb, lrq* request, SSHORT lck_wai // See if we've waited beyond the lock timeout - // if so we mark our own request as rejected - // !!! this will be changed to have no dependency on thread_db !!! - const bool cancelled = (tdbb->getCancelState() != FB_SUCCESS); + const bool cancelled = (callbacks.getCancelState() != FB_SUCCESS); if (cancelled || (lck_wait < 0 && lock_timeout <= current_time)) { @@ -3901,7 +3904,7 @@ void LockManager::wait_for_request(thread_db* tdbb, lrq* request, SSHORT lck_wai // We're going to do some real work - reset when we next want to // do a deadlock scan - deadlock_timeout = current_time + tdbb->adjustWait(scan_interval); + deadlock_timeout = current_time + callbacks.adjustWait(scan_interval); // Handle lock event first if (ret == FB_SUCCESS) @@ -3911,7 +3914,7 @@ void LockManager::wait_for_request(thread_db* tdbb, lrq* request, SSHORT lck_wai // This could happen if the lock was granted to a different request, // we have to tell the new owner of the lock that they are blocking us. - post_blockage(tdbb, request, lock); + post_blockage(callbacks, request, lock); continue; } @@ -3966,7 +3969,7 @@ void LockManager::wait_for_request(thread_db* tdbb, lrq* request, SSHORT lck_wai // We need to inform the new owner. DEBUG_MSG(0, ("wait_for_request: forcing a resignal of blockers\n")); - post_blockage(tdbb, request, lock); + post_blockage(callbacks, request, lock); #ifdef DEV_BUILD repost_counter++; if (repost_counter % 50 == 0) diff --git a/src/lock/lock_proto.h b/src/lock/lock_proto.h index a4f3219997d..e0bce331789 100644 --- a/src/lock/lock_proto.h +++ b/src/lock/lock_proto.h @@ -290,10 +290,20 @@ namespace Firebird { namespace Jrd { -class thread_db; - class LockManager final : public Firebird::GlobalStorage, public Firebird::IpcObject { +public: + class Callbacks + { + public: + virtual ~Callbacks() = default; + + virtual ISC_STATUS getCancelState() const = 0; + virtual ULONG adjustWait(ULONG wait) const = 0; + virtual void checkoutRun(std::function func) const = 0; + }; + +private: class LockTableGuard { public: @@ -395,15 +405,15 @@ class LockManager final : public Firebird::GlobalStorage, public Firebird::IpcOb ~LockManager(); bool initializeOwner(Firebird::CheckStatusWrapper*, LOCK_OWNER_T, UCHAR, SRQ_PTR*); - void shutdownOwner(thread_db*, SRQ_PTR*); + void shutdownOwner(const Callbacks&, SRQ_PTR*); - SRQ_PTR enqueue(thread_db*, Firebird::CheckStatusWrapper*, SRQ_PTR, const USHORT, + SRQ_PTR enqueue(const Callbacks&, Firebird::CheckStatusWrapper*, SRQ_PTR, const USHORT, const UCHAR*, const USHORT, UCHAR, lock_ast_t, void*, LOCK_DATA_T, SSHORT, SRQ_PTR); - bool convert(thread_db*, Firebird::CheckStatusWrapper*, SRQ_PTR, UCHAR, SSHORT, lock_ast_t, void*); - UCHAR downgrade(thread_db*, Firebird::CheckStatusWrapper*, const SRQ_PTR); + bool convert(const Callbacks&, Firebird::CheckStatusWrapper*, SRQ_PTR, UCHAR, SSHORT, lock_ast_t, void*); + UCHAR downgrade(const Callbacks&, Firebird::CheckStatusWrapper*, const SRQ_PTR); bool dequeue(const SRQ_PTR); - void repost(thread_db*, lock_ast_t, void*, SRQ_PTR); + void repost(const Callbacks&, lock_ast_t, void*, SRQ_PTR); bool cancelWait(SRQ_PTR); LOCK_DATA_T queryData(const USHORT, const USHORT); @@ -417,7 +427,7 @@ class LockManager final : public Firebird::GlobalStorage, public Firebird::IpcOb void acquire_shmem(SRQ_PTR); UCHAR* alloc(USHORT, Firebird::CheckStatusWrapper*); lbl* alloc_lock(USHORT, Firebird::CheckStatusWrapper*); - void blocking_action(thread_db*, SRQ_PTR); + void blocking_action(const Callbacks*, SRQ_PTR); void blocking_action_thread(); void bug(Firebird::CheckStatusWrapper*, const TEXT*); void bug_assert(const TEXT*, ULONG); @@ -430,15 +440,15 @@ class LockManager final : public Firebird::GlobalStorage, public Firebird::IpcOb lbl* find_lock(USHORT, const UCHAR*, USHORT, USHORT*); lrq* get_request(SRQ_PTR); void grant(lrq*, lbl*); - bool grant_or_que(thread_db*, lrq*, lbl*, SSHORT); + bool grant_or_que(const Callbacks&, lrq*, lbl*, SSHORT); bool init_owner_block(Firebird::CheckStatusWrapper*, own*, UCHAR, LOCK_OWNER_T); void insert_data_que(lbl*); void insert_tail(SRQ, SRQ); - bool internal_convert(thread_db* database, Firebird::CheckStatusWrapper*, SRQ_PTR, UCHAR, SSHORT, + bool internal_convert(const Callbacks&, Firebird::CheckStatusWrapper*, SRQ_PTR, UCHAR, SSHORT, lock_ast_t, void*); void internal_dequeue(SRQ_PTR); static USHORT lock_state(const lbl*); - void post_blockage(thread_db*, lrq*, lbl*); + void post_blockage(const Callbacks&, lrq*, lbl*); void post_history(USHORT, SRQ_PTR, SRQ_PTR, SRQ_PTR, bool); void post_pending(lbl*); void post_wakeup(own*); @@ -449,7 +459,7 @@ class LockManager final : public Firebird::GlobalStorage, public Firebird::IpcOb void remove_que(SRQ); void release_shmem(SRQ_PTR); void release_request(lrq*); - bool signal_owner(thread_db*, own*); + bool signal_owner(const Callbacks&, own*); void validate_history(const SRQ_PTR history_header); void validate_lhb(const lhb*); @@ -458,7 +468,7 @@ class LockManager final : public Firebird::GlobalStorage, public Firebird::IpcOb void validate_request(const SRQ_PTR, USHORT, USHORT); void validate_shb(const SRQ_PTR); - void wait_for_request(thread_db*, lrq*, SSHORT); + void wait_for_request(const Callbacks&, lrq*, SSHORT); bool init_shared_file(Firebird::CheckStatusWrapper*); void get_shared_file_name(Firebird::PathName&, ULONG extend = 0) const; From 3b228094d8f0bd4600344eb5f7bf4f94b5911300 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Thu, 28 Aug 2025 07:53:29 -0300 Subject: [PATCH 2/9] Add LockManager test disabled. --- builds/win32/msvc15/engine_test.vcxproj | 3 + .../win32/msvc15/engine_test.vcxproj.filters | 3 + src/jrd/tests/LockManagerTest.cpp | 124 ++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 src/jrd/tests/LockManagerTest.cpp diff --git a/builds/win32/msvc15/engine_test.vcxproj b/builds/win32/msvc15/engine_test.vcxproj index 0b36794b8e1..b50bcd18393 100644 --- a/builds/win32/msvc15/engine_test.vcxproj +++ b/builds/win32/msvc15/engine_test.vcxproj @@ -260,6 +260,9 @@ + + + diff --git a/builds/win32/msvc15/engine_test.vcxproj.filters b/builds/win32/msvc15/engine_test.vcxproj.filters index 5bfcdee7723..bbf8ba2e2ae 100644 --- a/builds/win32/msvc15/engine_test.vcxproj.filters +++ b/builds/win32/msvc15/engine_test.vcxproj.filters @@ -21,6 +21,9 @@ source + + source + source diff --git a/src/jrd/tests/LockManagerTest.cpp b/src/jrd/tests/LockManagerTest.cpp new file mode 100644 index 00000000000..3e2d5865125 --- /dev/null +++ b/src/jrd/tests/LockManagerTest.cpp @@ -0,0 +1,124 @@ +#include "firebird.h" +#include "boost/test/unit_test.hpp" +#include +#include +#include +#include +#include +#include +#include "../common/status.h" +#include "../common/classes/fb_string.h" +#include "../common/config/config.h" +#include "../lock/lock_proto.h" +#include "../jrd/lck.h" + +using namespace Firebird; +using namespace Jrd; + + +namespace +{ + class LockManagerTestCallbacks : public LockManager::Callbacks + { + public: + ISC_STATUS getCancelState() const + { + return 0; + } + + ULONG adjustWait(ULONG wait) const + { + return 0; + } + + void checkoutRun(std::function func) const + { + func(); + } + }; +} + + +static std::string getUniqueId() +{ + static std::atomic lockSuccess{0}; + + const auto now = std::chrono::system_clock::now(); + const auto nowNs = std::chrono::duration_cast(now.time_since_epoch()).count(); + + return "lm_" + + std::to_string(nowNs) + "_" + + std::to_string(lockSuccess.fetch_add(1)); +} + + +BOOST_AUTO_TEST_SUITE(EngineSuite) +BOOST_AUTO_TEST_SUITE(LockManagerSuite) +BOOST_AUTO_TEST_SUITE(LockManagerTests) + + +BOOST_AUTO_TEST_CASE(LockUnlockTest, *boost::unit_test::disabled()) +{ + constexpr unsigned THREAD_COUNT = 8u; + constexpr unsigned ITERATION_COUNT = 100'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); + + unsigned lockSuccess = 0u; + std::atomic_uint lockFail = 0; + + std::vector threads; + std::latch latch(THREAD_COUNT); + std::mutex mutex; + + for (unsigned threadNum = 0u; threadNum < THREAD_COUNT; ++threadNum) + { + threads.emplace_back([&]() { + const UCHAR LOCK_KEY[] = {'1'}; + 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 mutexGuard(mutex); + + const auto lockId = lockManager->enqueue(callbacks, &statusVector, 0, + LCK_expression, LOCK_KEY, sizeof(LOCK_KEY), LCK_EX, nullptr, nullptr, 0, LCK_WAIT, ownerHandle); + + if (lockId) + { + ++lockSuccess; + lockManager->dequeue(lockId); + } + else + ++lockFail; + } + + lockManager->shutdownOwner(callbacks, &ownerHandle); + }); + } + + for (auto& thread : threads) + thread.join(); + + BOOST_CHECK_EQUAL(lockFail + lockSuccess, THREAD_COUNT * ITERATION_COUNT); + // BOOST_CHECK_EQUAL(lockFail.load(), 0u); + // BOOST_CHECK_EQUAL(lockSuccess, 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 cc7e318342edc0d38aab7dcbc7f88672d06a45c9 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Thu, 28 Aug 2025 08:46:20 -0300 Subject: [PATCH 3/9] Correction --- src/jrd/tests/LockManagerTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jrd/tests/LockManagerTest.cpp b/src/jrd/tests/LockManagerTest.cpp index 3e2d5865125..727e0652175 100644 --- a/src/jrd/tests/LockManagerTest.cpp +++ b/src/jrd/tests/LockManagerTest.cpp @@ -28,7 +28,7 @@ namespace ULONG adjustWait(ULONG wait) const { - return 0; + return wait; } void checkoutRun(std::function func) const From 3dbfe3f1a53ec06086dcc97bfc9b7ca295fd713f Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Thu, 28 Aug 2025 20:19:24 -0300 Subject: [PATCH 4/9] Fix test, thanks to Vlad. --- src/jrd/tests/LockManagerTest.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/jrd/tests/LockManagerTest.cpp b/src/jrd/tests/LockManagerTest.cpp index 727e0652175..5985daca960 100644 --- a/src/jrd/tests/LockManagerTest.cpp +++ b/src/jrd/tests/LockManagerTest.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include #include "../common/status.h" @@ -74,11 +73,10 @@ BOOST_AUTO_TEST_CASE(LockUnlockTest, *boost::unit_test::disabled()) std::vector threads; std::latch latch(THREAD_COUNT); - std::mutex mutex; for (unsigned threadNum = 0u; threadNum < THREAD_COUNT; ++threadNum) { - threads.emplace_back([&]() { + threads.emplace_back([&, threadNum]() { const UCHAR LOCK_KEY[] = {'1'}; FbLocalStatus statusVector; LOCK_OWNER_T ownerId = threadNum + 1; @@ -90,8 +88,6 @@ BOOST_AUTO_TEST_CASE(LockUnlockTest, *boost::unit_test::disabled()) for (unsigned i = 0; i < ITERATION_COUNT; ++i) { - // std::lock_guard mutexGuard(mutex); - const auto lockId = lockManager->enqueue(callbacks, &statusVector, 0, LCK_expression, LOCK_KEY, sizeof(LOCK_KEY), LCK_EX, nullptr, nullptr, 0, LCK_WAIT, ownerHandle); @@ -111,9 +107,8 @@ BOOST_AUTO_TEST_CASE(LockUnlockTest, *boost::unit_test::disabled()) for (auto& thread : threads) thread.join(); - BOOST_CHECK_EQUAL(lockFail + lockSuccess, THREAD_COUNT * ITERATION_COUNT); - // BOOST_CHECK_EQUAL(lockFail.load(), 0u); - // BOOST_CHECK_EQUAL(lockSuccess, THREAD_COUNT * ITERATION_COUNT); + BOOST_CHECK_EQUAL(lockFail.load(), 0u); + BOOST_CHECK_EQUAL(lockSuccess, THREAD_COUNT * ITERATION_COUNT); lockManager.reset(); } From 60be52f70b0f6ab4f62437737e227bbedbd0edd9 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Thu, 28 Aug 2025 20:19:53 -0300 Subject: [PATCH 5/9] Enable test. --- src/jrd/tests/LockManagerTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jrd/tests/LockManagerTest.cpp b/src/jrd/tests/LockManagerTest.cpp index 5985daca960..c33391a50bc 100644 --- a/src/jrd/tests/LockManagerTest.cpp +++ b/src/jrd/tests/LockManagerTest.cpp @@ -56,7 +56,7 @@ BOOST_AUTO_TEST_SUITE(LockManagerSuite) BOOST_AUTO_TEST_SUITE(LockManagerTests) -BOOST_AUTO_TEST_CASE(LockUnlockTest, *boost::unit_test::disabled()) +BOOST_AUTO_TEST_CASE(LockUnlockTest) { constexpr unsigned THREAD_COUNT = 8u; constexpr unsigned ITERATION_COUNT = 100'000u; From 6d05b7c816b0ad1c36da76b297211cc03eec358f Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Thu, 28 Aug 2025 20:20:22 -0300 Subject: [PATCH 6/9] Make test run faster. --- src/jrd/tests/LockManagerTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jrd/tests/LockManagerTest.cpp b/src/jrd/tests/LockManagerTest.cpp index c33391a50bc..90d1e1bbaec 100644 --- a/src/jrd/tests/LockManagerTest.cpp +++ b/src/jrd/tests/LockManagerTest.cpp @@ -59,7 +59,7 @@ BOOST_AUTO_TEST_SUITE(LockManagerTests) BOOST_AUTO_TEST_CASE(LockUnlockTest) { 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); From 74ed4136ae8dc3b0909182a348d9a4f19bb1ce50 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Thu, 28 Aug 2025 21:20:09 -0300 Subject: [PATCH 7/9] Fix crash in Android build. --- src/lock/lock.cpp | 31 ++++++++++++++++++++++++++----- src/lock/lock_proto.h | 2 +- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/lock/lock.cpp b/src/lock/lock.cpp index 09ee3418a46..3739f3ce6af 100644 --- a/src/lock/lock.cpp +++ b/src/lock/lock.cpp @@ -1340,7 +1340,7 @@ lbl* LockManager::alloc_lock(USHORT length, CheckStatusWrapper* statusVector) } -void LockManager::blocking_action(const Callbacks* callbacks, +void LockManager::blocking_action(const Callbacks& callbacks, SRQ_PTR blocking_owner_offset) { /************************************** @@ -1399,8 +1399,7 @@ void LockManager::blocking_action(const Callbacks* callbacks, { // checkout scope LockTableCheckout checkout(this, FB_FUNCTION); - fb_assert(callbacks); - callbacks->checkoutRun([&] { + callbacks.checkoutRun([&] { try { (*routine)(arg); @@ -1488,7 +1487,29 @@ void LockManager::blocking_action_thread() { const SRQ_PTR owner_offset = SRQ_REL_PTR(owner); guard.setOwner(owner_offset); - blocking_action(nullptr, owner_offset); + + class BlockingActionCallbacks final : public Callbacks + { + public: + ISC_STATUS getCancelState() const override + { + fb_assert(false); + return 0; + } + + ULONG adjustWait(ULONG wait) const override + { + fb_assert(false); + return wait; + } + + void checkoutRun(std::function func) const override + { + func(); + } + }; + + blocking_action(BlockingActionCallbacks(), owner_offset); completed = false; break; } @@ -3205,7 +3226,7 @@ bool LockManager::signal_owner(const Callbacks& callbacks, own* blocking_owner) if (process->prc_process_id == PID) { DEBUG_DELAY; - blocking_action(&callbacks, SRQ_REL_PTR(blocking_owner)); + blocking_action(callbacks, SRQ_REL_PTR(blocking_owner)); DEBUG_DELAY; return true; } diff --git a/src/lock/lock_proto.h b/src/lock/lock_proto.h index e0bce331789..b0b14511938 100644 --- a/src/lock/lock_proto.h +++ b/src/lock/lock_proto.h @@ -427,7 +427,7 @@ class LockManager final : public Firebird::GlobalStorage, public Firebird::IpcOb void acquire_shmem(SRQ_PTR); UCHAR* alloc(USHORT, Firebird::CheckStatusWrapper*); lbl* alloc_lock(USHORT, Firebird::CheckStatusWrapper*); - void blocking_action(const Callbacks*, SRQ_PTR); + void blocking_action(const Callbacks&, SRQ_PTR); void blocking_action_thread(); void bug(Firebird::CheckStatusWrapper*, const TEXT*); void bug_assert(const TEXT*, ULONG); From e7612a048fafa320db59f6c0fa459e5709ae06ad Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Thu, 28 Aug 2025 22:50:10 -0300 Subject: [PATCH 8/9] Move LockManagerTest to src/lock/tests --- builds/posix/make.shared.variables | 2 +- builds/win32/msvc15/engine_test.vcxproj | 4 ++-- builds/win32/msvc15/engine_test.vcxproj.filters | 4 ++-- src/{jrd => lock}/tests/LockManagerTest.cpp | 0 4 files changed, 5 insertions(+), 5 deletions(-) rename src/{jrd => lock}/tests/LockManagerTest.cpp (100%) diff --git a/builds/posix/make.shared.variables b/builds/posix/make.shared.variables index 16fce089b02..f8d15e06879 100644 --- a/builds/posix/make.shared.variables +++ b/builds/posix/make.shared.variables @@ -92,7 +92,7 @@ Engine_Objects:= $(call dirObjects,jrd) $(call dirObjects,dsql) $(call dirObject $(call dirObjects,jrd/sys-packages) $(call dirObjects,jrd/trace) \ $(call makeObjects,lock,lock.cpp) -Engine_Test_Objects:= $(call dirObjects,jrd/tests) +Engine_Test_Objects:= $(call dirObjects,jrd/tests) $(call dirObjects,lock/tests) AllObjects += $(Engine_Objects) $(Engine_Test_Objects) diff --git a/builds/win32/msvc15/engine_test.vcxproj b/builds/win32/msvc15/engine_test.vcxproj index b50bcd18393..cba0b0c6a13 100644 --- a/builds/win32/msvc15/engine_test.vcxproj +++ b/builds/win32/msvc15/engine_test.vcxproj @@ -261,10 +261,10 @@ - + - + diff --git a/builds/win32/msvc15/engine_test.vcxproj.filters b/builds/win32/msvc15/engine_test.vcxproj.filters index bbf8ba2e2ae..6dd45c98d06 100644 --- a/builds/win32/msvc15/engine_test.vcxproj.filters +++ b/builds/win32/msvc15/engine_test.vcxproj.filters @@ -21,10 +21,10 @@ source - + source - + source diff --git a/src/jrd/tests/LockManagerTest.cpp b/src/lock/tests/LockManagerTest.cpp similarity index 100% rename from src/jrd/tests/LockManagerTest.cpp rename to src/lock/tests/LockManagerTest.cpp From 76e4a03c11ad1438535e10a28f2806af126214d4 Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes Date: Sun, 31 Aug 2025 09:37:42 -0300 Subject: [PATCH 9/9] More LockManager test. --- src/lock/tests/LockManagerTest.cpp | 61 +++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/src/lock/tests/LockManagerTest.cpp b/src/lock/tests/LockManagerTest.cpp index 90d1e1bbaec..4ac2ee5ff66 100644 --- a/src/lock/tests/LockManagerTest.cpp +++ b/src/lock/tests/LockManagerTest.cpp @@ -56,7 +56,7 @@ BOOST_AUTO_TEST_SUITE(LockManagerSuite) BOOST_AUTO_TEST_SUITE(LockManagerTests) -BOOST_AUTO_TEST_CASE(LockUnlockTest) +BOOST_AUTO_TEST_CASE(LockUnlockWaitTest) { constexpr unsigned THREAD_COUNT = 8u; constexpr unsigned ITERATION_COUNT = 10'000u; @@ -114,6 +114,65 @@ BOOST_AUTO_TEST_CASE(LockUnlockTest) } +BOOST_AUTO_TEST_CASE(LockUnlockNoWaitTest) +{ + 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); + + unsigned lockSuccess = 0u; + std::atomic_uint lockFail = 0; + + std::vector threads; + std::latch latch(THREAD_COUNT); + + for (unsigned threadNum = 0u; threadNum < THREAD_COUNT; ++threadNum) + { + threads.emplace_back([&, threadNum]() { + const UCHAR LOCK_KEY[] = {'1'}; + 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) + { + const auto lockId = lockManager->enqueue(callbacks, &statusVector, 0, + LCK_expression, LOCK_KEY, sizeof(LOCK_KEY), LCK_EX, nullptr, nullptr, 0, LCK_NO_WAIT, ownerHandle); + + if (lockId) + { + ++lockSuccess; + lockManager->dequeue(lockId); + } + else + ++lockFail; + } + + lockManager->shutdownOwner(callbacks, &ownerHandle); + }); + } + + for (auto& thread : threads) + thread.join(); + + BOOST_CHECK_GT(lockFail.load(), 0u); + BOOST_CHECK_GT(lockSuccess, 0u); + BOOST_CHECK_EQUAL(lockSuccess + lockFail, THREAD_COUNT * ITERATION_COUNT); + + lockManager.reset(); +} + + BOOST_AUTO_TEST_SUITE_END() // LockManagerTests BOOST_AUTO_TEST_SUITE_END() // LockManagerSuite BOOST_AUTO_TEST_SUITE_END() // EngineSuite