Changes in LockManager to make it testable in isolation

[Adriano dos Santos Fernandes]

Hi!

I propose a change in LockManager class removing its dependency in
thread_db.

It would be done passing around an interface like this:

class Callbacks
{
public:
virtual ISC_STATUS getCancelState() const = 0;
virtual ULONG adjustWait(ULONG wait) const = 0;
virtual void checkoutRun(std::function<void()> func) const = 0;
};

With this interface, I'm able to create isolated tested without a
database, which is invaluable tool to stress test multi-thread issues.

Would it be acceptable?

(I may even have found something weird (not sure) already, were a
multi-thread test works in debug mode but does not in release).


Adriano

[Adriano dos Santos Fernandes]

On 8/26/25 22:58, Adriano dos Santos Fernandes wrote:
>
> (I may even have found something weird (not sure) already, were a
> multi-thread test works in debug mode but does not in release).
>

And here is a snippet of kind of test I want to write which such refactor:

----
std::vector<std::thread> 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();
----

With the mutexGuard commented, it consistently hangs or calls
LockManager::bug.


Adriano

[Vlad Khorsun]

Interesting to see, push it to the separate branch, please.

Regards,
Vlad

[Adriano dos Santos Fernandes]

https://github.com/FirebirdSQL/firebird/pull/8717


Adriano

[Vlad Khorsun]

28.08.2025 3:44, Adriano dos Santos Fernandes:

Thanks, looks OK for me.

Could you also put your mt-test there ?

Regards,
Vlad

[Adriano dos Santos Fernandes]

I added it disabled to avoid hang the CI build.

Note that normally the LM would be calling checkoutRun that checkouts
from the engine and in the test it do nothing.


Adriano