From f2fac63e086b4bd8dfc3ed9c9f31241eb9df9b9c Mon Sep 17 00:00:00 2001 From: Christian Mollekopf Date: Mon, 6 Aug 2018 13:33:53 +0200 Subject: Fixed threading issues with dynamic db creation. This patch addresses two problems: * A potential deadlock. We had the following code inside a separately protected section: dbiLocker.unlock(); //Here we could loos the readlock QWriteLocker dbiWriteLocker(&sDbisLock); If we lost the lock in between the two lines, the second thread that was now holding a read-lock on sDbisLock could not enter the protected section, which was a requirement to release the read-lock, and we'd thus end up in a deadlock. This is solved using tryLock with intermediate releases of the read-lock, allowing the original thread to finish. * When failing to validate a dbi for the current transacation we simply returned an invalid db (which then in this particular case broke reading of revision uid's and type's), leading to queries not executing as they should. Both problems are unfortunately hard to reproduce, the adjusted test at leaset allowed me to reproduce the deadlock situation sometimes. To fix this cleanly we should probably just get rid of dynamic dbi allocation for good. --- tests/storagetest.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/storagetest.cpp b/tests/storagetest.cpp index 4c44322..81acc13 100644 --- a/tests/storagetest.cpp +++ b/tests/storagetest.cpp @@ -191,14 +191,16 @@ private slots: // We repeat the test a bunch of times since failing is relatively random for (int tries = 0; tries < 10; tries++) { + //clearEnv in combination with the bogus db layouts tests the dynamic named db opening as well. + Sink::Storage::DataStore::clearEnv(); bool error = false; // Try to concurrently read QList> futures; const int concurrencyLevel = 20; for (int num = 0; num < concurrencyLevel; num++) { futures << QtConcurrent::run([this, &error]() { - Sink::Storage::DataStore storage(testDataPath, dbName, Sink::Storage::DataStore::ReadOnly); - Sink::Storage::DataStore storage2(testDataPath, dbName + "2", Sink::Storage::DataStore::ReadOnly); + Sink::Storage::DataStore storage(testDataPath, {dbName, {{"bogus", 0}}}, Sink::Storage::DataStore::ReadOnly); + Sink::Storage::DataStore storage2(testDataPath, {dbName+ "2", {{"bogus", 0}}}, Sink::Storage::DataStore::ReadOnly); for (int i = 0; i < count; i++) { if (!verify(storage, i)) { error = true; @@ -757,7 +759,7 @@ private slots: Sink::Storage::DataStore::clearEnv(); //Try to read-write dynamic opening of the db. - //This is the case if we don't have all databases available upon initializatoin and we don't (e.g. because the db hasn't been created yet) + //This is the case if we don't have all databases available upon initialization and we don't (e.g. because the db hasn't been created yet) { // Trick the db into not loading all dbs by passing in a bogus layout. Sink::Storage::DataStore store(testDataPath, {dbName, {{"bogus", 0}}}, Sink::Storage::DataStore::ReadWrite); -- cgit v1.2.3