Commit 80aab523 authored by Milian Wolff's avatar Milian Wolff
Browse files

Refactor ItemRepository code to make it work without recursive mutex

While the IndexedString used a repository with a non-recursive mutex,
this was not a setup that worked in general: removing an item e.g.
would deadlock in such a scenario. IndexedString got lucky because
it never tries to delete anything from the repository...

Refactor the code as follows:

The bulk of what was ItemRepository is now ItemRepositoryPrivate
and assumes that it gets accessed with external serialization.

The new ItemRepository is just a shim around that Private but it
provides the serialization by using the ThisLocker as before.

This way, the Private class can call its own API in any way it wants
and we don't run into a situation where we would accidentally deadlock
by relocking a non-recursive mutex.

For now, only the tests are using the non-recursive mutex now. Other
repositories will be ported one-by-one now.
parent f89ce6f3
This diff is collapsed.
......@@ -123,7 +123,7 @@ static QVector<uint> insertData(const QVector<QString>& data, TestDataRepository
void BenchItemRepository::insert()
{
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
TestDataRepository repo("TestDataRepositoryInsert", &mutex);
const QVector<QString> data = generateData();
QVector<uint> indices;
......@@ -137,7 +137,7 @@ void BenchItemRepository::insert()
void BenchItemRepository::remove()
{
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
TestDataRepository repo("TestDataRepositoryRemove", &mutex);
const QVector<QString> data = generateData();
const QVector<uint> indices = insertData(data, repo);
......@@ -162,7 +162,7 @@ void BenchItemRepository::removeDisk()
{
const QVector<QString> data = generateData();
QVector<uint> indices;
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
{
TestDataRepository repo("TestDataRepositoryRemoveDisk", &mutex);
indices = insertData(data, repo);
......@@ -182,7 +182,7 @@ void BenchItemRepository::removeDisk()
void BenchItemRepository::lookupKey()
{
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
TestDataRepository repo("TestDataRepositoryLookupKey", &mutex);
const QVector<QString> data = generateData();
QVector<uint> indices = insertData(data, repo);
......@@ -196,7 +196,7 @@ void BenchItemRepository::lookupKey()
void BenchItemRepository::lookupValue()
{
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
TestDataRepository repo("TestDataRepositoryLookupValue", &mutex);
const QVector<QString> data = generateData();
QVector<uint> indices = insertData(data, repo);
......
......@@ -119,7 +119,7 @@ class TestItemRepository
private Q_SLOTS:
void testItemRepository()
{
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
ItemRepository<TestItem, TestItemRequest> repository(QStringLiteral("TestItemRepository"), &mutex);
uint itemId = 0;
QHash<uint, TestItem*> realItemsByIndex;
......@@ -213,7 +213,7 @@ private Q_SLOTS:
}
void testLeaks()
{
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
ItemRepository<TestItem, TestItemRequest> repository(QStringLiteral("TestItemRepository"), &mutex);
QList<TestItem*> items;
for (int i = 0; i < 10000; ++i) {
......@@ -241,7 +241,7 @@ private Q_SLOTS:
}
void deleteClashingMonsterBucket()
{
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
ItemRepository<TestItem, TestItemRequest> repository(QStringLiteral("TestItemRepository"), &mutex);
const uint hash = 1235;
......@@ -273,11 +273,11 @@ private Q_SLOTS:
}
void usePermissiveModuloWhenRemovingClashLinks()
{
auto mutex = QMutex(QMutex::Recursive);
QMutex mutex;
ItemRepository<TestItem, TestItemRequest> repository(QStringLiteral("PermissiveModulo"), &mutex);
const uint bucketHashSize = decltype(repository)::bucketHashSize;
const uint nextBucketHashSize = decltype(repository)::MyBucket::NextBucketHashSize;
const uint bucketHashSize = decltype(repository)::Private::bucketHashSize;
const uint nextBucketHashSize = decltype(repository)::Private::MyBucket::NextBucketHashSize;
auto bucketNumberForIndex = [](const uint index) {
return index >> 16;
};
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment