From b73ae80dcd88e245dc1fa83c06f1d5356ea39196 Mon Sep 17 00:00:00 2001
From: Christian Mollekopf <chrigi_1@fastmail.fm>
Date: Wed, 11 Jul 2018 21:17:41 +0200
Subject: 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.
---
 common/storage_lmdb.cpp | 12 ++++++++++--
 tests/storagetest.cpp   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-
 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:
         QReadLocker dbiLocker{&sDbisLock};
         if (sDbis.contains(dbiName)) {
             dbi = sDbis.value(dbiName);
-            Q_ASSERT(dbiValidForTransaction(dbi, transaction));
+            //sDbis can potentially contain a dbi that is not valid for this transaction, if this transaction was created before the dbi was created.
+            if (!dbiValidForTransaction(dbi, transaction)) {
+                SinkTrace() << "Found dbi that is not available for the current transaction.";
+                return false;
+            }
         } else {
             /*
              * Dynamic creation of databases.
@@ -250,7 +254,11 @@ public:
             //Double checked locking
             if (sDbis.contains(dbiName)) {
                 dbi = sDbis.value(dbiName);
-                Q_ASSERT(dbiValidForTransaction(dbi, transaction));
+                //sDbis can potentially contain a dbi that is not valid for this transaction, if this transaction was created before the dbi was created.
+                if (!dbiValidForTransaction(dbi, transaction)) {
+                    SinkTrace() << "Found dbi that is not available for the current transaction.";
+                    return false;
+                }
                 return true;
             }
 
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:
     {
         bool gotResult = false;
         bool gotError = false;
+        Sink::Storage::DataStore store(testDataPath, {dbName, {{"test", 0}}}, Sink::Storage::DataStore::ReadOnly);
+        QVERIFY(!store.exists());
+        auto transaction = store.createTransaction(Sink::Storage::DataStore::ReadOnly);
+        Sink::Storage::DataStore::getUids("test", transaction, [&](const QByteArray &uid) {});
+        int numValues = transaction
+                            .openDatabase("test")
+                            .scan("",
+                                [&](const QByteArray &key, const QByteArray &value) -> bool {
+                                    gotResult = true;
+                                    return false;
+                                },
+                                [&](const Sink::Storage::DataStore::Error &error) {
+                                    qDebug() << error.message;
+                                    gotError = true;
+                                });
+        QCOMPARE(numValues, 0);
+        QVERIFY(!gotResult);
+        QVERIFY(!gotError);
+    }
+
+    /*
+     * This scenario tests a very specific pattern that can appear with new named databases.
+     * * A read-only transaction is opened
+     * * A write-transaction creates a new named db.
+     * * We try to access that named-db from the already open transaction.
+     */
+    void testNewDbInOpenTransaction()
+    {
+        //Create env, otherwise we don't even get a transaction
+        {
+            Sink::Storage::DataStore store(testDataPath, dbName, Sink::Storage::DataStore::ReadWrite);
+            auto transaction = store.createTransaction(Sink::Storage::DataStore::ReadWrite);
+        }
+        //Open a longlived transaction
         Sink::Storage::DataStore store(testDataPath, dbName, Sink::Storage::DataStore::ReadOnly);
-        int numValues = store.createTransaction(Sink::Storage::DataStore::ReadOnly)
+        auto transaction = store.createTransaction(Sink::Storage::DataStore::ReadOnly);
+
+        //Create the named database
+        {
+            Sink::Storage::DataStore store(testDataPath, dbName, Sink::Storage::DataStore::ReadWrite);
+            auto transaction = store.createTransaction(Sink::Storage::DataStore::ReadWrite);
+            transaction.openDatabase("test");
+            transaction.commit();
+        }
+
+
+        //Try to access the named database in the existing transaction. Opening should fail.
+        bool gotResult = false;
+        bool gotError = false;
+        int numValues = transaction
                             .openDatabase("test")
                             .scan("",
                                 [&](const QByteArray &key, const QByteArray &value) -> bool {
-- 
cgit v1.2.3