summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristian Mollekopf <chrigi_1@fastmail.fm>2018-07-11 21:17:41 +0200
committerChristian Mollekopf <chrigi_1@fastmail.fm>2018-07-12 09:04:44 +0200
commitb73ae80dcd88e245dc1fa83c06f1d5356ea39196 (patch)
tree308da8e4c2501e937a7b8f50f4eaecc1d30b245d
parent56974b62bd07d844d7cf578722a24962ecefd8b4 (diff)
downloadsink-b73ae80dcd88e245dc1fa83c06f1d5356ea39196.tar.gz
sink-b73ae80dcd88e245dc1fa83c06f1d5356ea39196.zip
Fixed the case when a dbi would leak through to a transaction where it
shouldn't be visible yet. Was reproducible in the initial sync of the caldav resource.
-rw-r--r--common/storage_lmdb.cpp12
-rw-r--r--tests/storagetest.cpp50
2 files changed, 59 insertions, 3 deletions
diff --git a/common/storage_lmdb.cpp b/common/storage_lmdb.cpp
index fe3b303..f254077 100644
--- a/common/storage_lmdb.cpp
+++ b/common/storage_lmdb.cpp
@@ -222,7 +222,11 @@ public:
222 QReadLocker dbiLocker{&sDbisLock}; 222 QReadLocker dbiLocker{&sDbisLock};
223 if (sDbis.contains(dbiName)) { 223 if (sDbis.contains(dbiName)) {
224 dbi = sDbis.value(dbiName); 224 dbi = sDbis.value(dbiName);
225 Q_ASSERT(dbiValidForTransaction(dbi, transaction)); 225 //sDbis can potentially contain a dbi that is not valid for this transaction, if this transaction was created before the dbi was created.
226 if (!dbiValidForTransaction(dbi, transaction)) {
227 SinkTrace() << "Found dbi that is not available for the current transaction.";
228 return false;
229 }
226 } else { 230 } else {
227 /* 231 /*
228 * Dynamic creation of databases. 232 * Dynamic creation of databases.
@@ -250,7 +254,11 @@ public:
250 //Double checked locking 254 //Double checked locking
251 if (sDbis.contains(dbiName)) { 255 if (sDbis.contains(dbiName)) {
252 dbi = sDbis.value(dbiName); 256 dbi = sDbis.value(dbiName);
253 Q_ASSERT(dbiValidForTransaction(dbi, transaction)); 257 //sDbis can potentially contain a dbi that is not valid for this transaction, if this transaction was created before the dbi was created.
258 if (!dbiValidForTransaction(dbi, transaction)) {
259 SinkTrace() << "Found dbi that is not available for the current transaction.";
260 return false;
261 }
254 return true; 262 return true;
255 } 263 }
256 264
diff --git a/tests/storagetest.cpp b/tests/storagetest.cpp
index bca91b1..f4cf400 100644
--- a/tests/storagetest.cpp
+++ b/tests/storagetest.cpp
@@ -275,8 +275,56 @@ private slots:
275 { 275 {
276 bool gotResult = false; 276 bool gotResult = false;
277 bool gotError = false; 277 bool gotError = false;
278 Sink::Storage::DataStore store(testDataPath, {dbName, {{"test", 0}}}, Sink::Storage::DataStore::ReadOnly);
279 QVERIFY(!store.exists());
280 auto transaction = store.createTransaction(Sink::Storage::DataStore::ReadOnly);
281 Sink::Storage::DataStore::getUids("test", transaction, [&](const QByteArray &uid) {});
282 int numValues = transaction
283 .openDatabase("test")
284 .scan("",
285 [&](const QByteArray &key, const QByteArray &value) -> bool {
286 gotResult = true;
287 return false;
288 },
289 [&](const Sink::Storage::DataStore::Error &error) {
290 qDebug() << error.message;
291 gotError = true;
292 });
293 QCOMPARE(numValues, 0);
294 QVERIFY(!gotResult);
295 QVERIFY(!gotError);
296 }
297
298 /*
299 * This scenario tests a very specific pattern that can appear with new named databases.
300 * * A read-only transaction is opened
301 * * A write-transaction creates a new named db.
302 * * We try to access that named-db from the already open transaction.
303 */
304 void testNewDbInOpenTransaction()
305 {
306 //Create env, otherwise we don't even get a transaction
307 {
308 Sink::Storage::DataStore store(testDataPath, dbName, Sink::Storage::DataStore::ReadWrite);
309 auto transaction = store.createTransaction(Sink::Storage::DataStore::ReadWrite);
310 }
311 //Open a longlived transaction
278 Sink::Storage::DataStore store(testDataPath, dbName, Sink::Storage::DataStore::ReadOnly); 312 Sink::Storage::DataStore store(testDataPath, dbName, Sink::Storage::DataStore::ReadOnly);
279 int numValues = store.createTransaction(Sink::Storage::DataStore::ReadOnly) 313 auto transaction = store.createTransaction(Sink::Storage::DataStore::ReadOnly);
314
315 //Create the named database
316 {
317 Sink::Storage::DataStore store(testDataPath, dbName, Sink::Storage::DataStore::ReadWrite);
318 auto transaction = store.createTransaction(Sink::Storage::DataStore::ReadWrite);
319 transaction.openDatabase("test");
320 transaction.commit();
321 }
322
323
324 //Try to access the named database in the existing transaction. Opening should fail.
325 bool gotResult = false;
326 bool gotError = false;
327 int numValues = transaction
280 .openDatabase("test") 328 .openDatabase("test")
281 .scan("", 329 .scan("",
282 [&](const QByteArray &key, const QByteArray &value) -> bool { 330 [&](const QByteArray &key, const QByteArray &value) -> bool {