diff options
author | Christian Mollekopf <chrigi_1@fastmail.fm> | 2018-07-11 21:17:41 +0200 |
---|---|---|
committer | Christian Mollekopf <chrigi_1@fastmail.fm> | 2018-07-12 09:04:44 +0200 |
commit | b73ae80dcd88e245dc1fa83c06f1d5356ea39196 (patch) | |
tree | 308da8e4c2501e937a7b8f50f4eaecc1d30b245d | |
parent | 56974b62bd07d844d7cf578722a24962ecefd8b4 (diff) | |
download | sink-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.cpp | 12 | ||||
-rw-r--r-- | 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: | |||
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 { |