Merge pull request #1575 from ispyisail/fix/1571-element-cache-lock

Fix #1571: collection freezes while another QElectroTech is open

sources/elementscollectioncache.cpp

@@ -40,6 +40,11 @@ ElementsCollectionCache::ElementsCollectionCache(const QString &database_path, Q
 	QString connection_name = QString("ElementsCollectionCache-%1").arg(cache_instances++);
 	cache_db_ = QSqlDatabase::addDatabase("QSQLITE", connection_name);
 	cache_db_.setDatabaseName(database_path);
+	// Every QElectroTech on this computer shares this file: two builds in
+	// different folders, or a build beside a released version, both run.
+	// The cache only saves time, so never wait for another one to let go
+	// of it -- a busy file is a cache miss (#1571).
+	cache_db_.setConnectOptions("QSQLITE_BUSY_TIMEOUT=0");
 
 	if (!cache_db_.open())
 		qDebug() << "Unable to open the SQLite database " << database_path << " as " << connection_name << ": " << cache_db_.lastError();
@@ -49,7 +54,6 @@ ElementsCollectionCache::ElementsCollectionCache(const QString &database_path, Q
 		QSqlQuery(cache_db_).exec("PRAGMA journal_mode = MEMORY");
 		QSqlQuery(cache_db_).exec("PRAGMA page_size = 4096");
 		QSqlQuery(cache_db_).exec("PRAGMA cache_size = 16384");
-		QSqlQuery(cache_db_).exec("PRAGMA locking_mode = EXCLUSIVE");
 		QSqlQuery(cache_db_).exec("PRAGMA synchronous = OFF");
 
 			// Previews used to be stored on an opaque white sheet; they are
@@ -301,7 +305,7 @@ bool ElementsCollectionCache::cacheName(const QString &path,
 	insert_name_ -> bindValue(":name",   current_name_);
 	if (!insert_name_ -> exec())
 	{
-		qDebug() << cache_db_.lastError();
+		qDebug() << insert_name_ -> lastError();
 		return(false);
 	}
 	return(true);
@@ -327,7 +331,7 @@ bool ElementsCollectionCache::cachePixmap(const QString &path,
 	insert_pixmap_ -> bindValue(":pixmap", QVariant(ba));
 	if (!insert_pixmap_->exec())
 	{
-		qDebug() << cache_db_.lastError();
+		qDebug() << insert_pixmap_ -> lastError();
 		return(false);
 	}
 	return(true);

tests/qttest/CMakeLists.txt

@@ -1494,3 +1494,7 @@ qet_add_app_test(tst_elementspanelclosedproject tst_elementspanelclosedproject.c
 # A project closed before the element collection panel's first load, or during
 # a load, stayed in the collection and the next reload read the freed project.
 qet_add_app_test(tst_collectionclosedproject tst_collectionclosedproject.cpp)
+
+# A second QElectroTech writes to the shared element cache at once instead of
+# waiting for the first to close (#1571).
+qet_add_app_test(tst_elementcachelock tst_elementcachelock.cpp)

tests/qttest/tst_elementcachelock.cpp

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <QtTest>
+
+#include <QElapsedTimer>
+#include <QTemporaryDir>
+#include <QUuid>
+
+#include "elementscollectioncache.h"
+
+// Every QElectroTech on a computer shares one elements_cache.sqlite. The cache
+// used PRAGMA locking_mode = EXCLUSIVE, so a QElectroTech that had once read
+// the file kept it locked until it closed, and every write from a second one
+// waited SQLite's 5 s busy timeout and failed -- on the GUI thread, twice per
+// symbol shown in the collection (#1571). Two caches on the same file in one
+// process lock each other exactly as two processes do.
+class tst_elementcachelock : public QObject
+{
+	Q_OBJECT
+
+	QTemporaryDir m_dir;
+	const QUuid m_uuid = QUuid::createUuid();
+
+private slots:
+	void secondCacheWritesWithoutWaiting()
+	{
+		QVERIFY(m_dir.isValid());
+		const QString path = m_dir.filePath(QStringLiteral("elements_cache.sqlite"));
+
+		ElementsCollectionCache first(path);
+		QVERIFY(first.cacheName(QStringLiteral("embed://a.elmt"), m_uuid));
+		QVERIFY(first.fetchNameFromCache(QStringLiteral("embed://a.elmt"), m_uuid));
+		QVERIFY(!first.fetchNameFromCache(QStringLiteral("embed://missing.elmt"), m_uuid));
+
+		ElementsCollectionCache second(path);
+		QElapsedTimer timer;
+		timer.start();
+		QVERIFY(second.cacheName(QStringLiteral("embed://b.elmt"), m_uuid));
+		QVERIFY(second.cachePixmap(QStringLiteral("embed://b.elmt"), m_uuid));
+		QVERIFY2(timer.elapsed() < 1000, qPrintable(QString::number(timer.elapsed()) + " ms"));
+
+		// and each one sees what the other wrote
+		QVERIFY(second.fetchNameFromCache(QStringLiteral("embed://a.elmt"), m_uuid));
+		QVERIFY(first.fetchNameFromCache(QStringLiteral("embed://b.elmt"), m_uuid));
+		QVERIFY(first.cacheName(QStringLiteral("embed://c.elmt"), m_uuid));
+	}
+};
+
+QTEST_MAIN(tst_elementcachelock)
+#include "tst_elementcachelock.moc"