Protect database creation by a file lock.

Otherwise, due to SQLITE locking rules, database creation may fail late,
disturbing integration tests among others.

Author: tycho-kirchner

src/common/database/db_connection.cpp

@@ -11,9 +11,11 @@
 #include <unistd.h>
 
 #include "db_connection.h"
+#include "cflock.h"
 #include "sqlite_database_scheme.h"
 #include "sqlite_database_scheme_updates.h"
 #include "qexcdatabase.h"
+#include "qfilethrow.h"
 #include "qsqlquerythrow.h"
 #include "logger.h"
 #include "app.h"
@@ -22,6 +24,12 @@
 
 static QSqlDatabase* g_db = nullptr;
 
+static bool versionTableExists(QSqlQueryThrow& query){
+    logDebug << "checking for version table...";
+    query.exec("SELECT name FROM sqlite_master WHERE type='table' AND name='version'");
+    return query.next();
+}
+
 static QVersionNumber queryVersion(QSqlQueryThrow& query){
     query.exec("select ver from version");
     query.next(true);
@@ -88,55 +96,76 @@ static void handleDifferentVersions(const QVersionNumber& dbVersion,
 
 }
 
-/// @param versionAction: if true, update version, if needed
-/// @throws QExcDatabase
-static void openAndPrepareSqliteDb()
-{
-    const QString appDataLoc = db_connection::mkDbPath();
-    const QString dbPath = appDataLoc + "/database.db";
-
-    bool pathExisted = QFileInfo::exists(dbPath);
-    g_db->setDatabaseName(dbPath);
-    if(! g_db->open()) {
-        throw QExcDatabase(__func__, g_db->lastError());
-    }
-
-    QSqlQueryThrow query(*g_db);
-    // Allow for delete queries with cascades
-
+static void
+createOrUpDateDb(QSqlQueryThrow &query, CFlock& lock){
+    logDebug << "about to lockExclusive database for scheme update...";
     // quoting sqlite.org/foreignkeys.html
     // "It is not possible to enable or disable foreign key constraints in the
     //  middle of a multi-statement transaction (when SQLite is not in autocommit mode)"
-    // The scheme-updates require foreign_keys=OFF, so enable below
+    // The scheme-updates require foreign_keys=OFF, so call below pragma:
     query.exec("PRAGMA foreign_keys=OFF");
-
+    lock.lockExclusive();
     query.transaction();
 
-    if(! pathExisted){
-        // Initially create the database
+    if(! versionTableExists(query)){
         logInfo << qtr("Creating new sqlite database");
-        QStringList statements = QString(SQLITE_DATABASE_SCHEME).split(';', QString::SkipEmptyParts);
-
+        QStringList statements = QString(
+                    SQLITE_DATABASE_SCHEME).split(';', QString::SkipEmptyParts);
         for(const QString& stmt : statements){
             query.exec(stmt);
         }
-        QFile dbfile(appDataLoc);
-        if(! dbfile.setPermissions(
-                    QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner)){
-            logWarning << qtr("Failed to initially set permissions on the database-directory "
-                              "at %1: %2. Other users might be able "
+
+        QFile dbDir(db_connection::getDatabaseDir());
+        if(! dbDir.setPermissions(
+             QFileDevice::ReadOwner|QFileDevice::WriteOwner|QFileDevice::ExeOwner)){
+            logWarning << qtr("Failed to initially set permissions on the database-"
+                              "directory at %1: %2. Other users might be able "
                               "to browse your command history...")
-                          .arg(dbPath, dbfile.errorString());
+                          .arg(db_connection::getDatabaseDir(), dbDir.errorString());
         }
     }
     const auto dbVersion = queryVersion(query);
-    logDebug << "current db-version" << dbVersion.toString();
     if(dbVersion != app::version()){
         handleDifferentVersions(dbVersion, query);
     }
-
     query.commit();
     // outside of transaction (see above):
+    // Allow for delete queries with cascades
+    query.exec("PRAGMA foreign_keys=ON");
+}
+
+/// @throws QExcDatabase
+static void openAndPrepareSqliteDb()
+{
+    const QString appDataLoc = db_connection::mkDbPath();
+    const QString dbPath = appDataLoc + "/database.db";
+    QFileThrow lockfile(appDataLoc + "/.shournal-dblock");
+    lockfile.open(QFile::OpenModeFlag::ReadWrite);
+    // Lock to allow for concurrent database scheme updates. First, we lock shared,
+    // upgrading to exclusive on scheme update. Note that for some reason concurrent
+    // processes executing "PRAGMA locking_mode=EXCLUSIVE; BEGIN EXCLUSIVE;" deadlocked
+    // during integration tests, so be careful with that directive.
+    CFlock lock(lockfile.handle());
+    lock.lockShared();
+    g_db->setDatabaseName(dbPath);
+    if(! g_db->open()) {
+        throw QExcDatabase(__func__, g_db->lastError());
+    }
+
+    QSqlQueryThrow query(*g_db);
+    if(! versionTableExists(query)){
+        logDebug << "version table did not exist yet..";
+        createOrUpDateDb(query, lock);
+    } else {
+        const auto dbVersion = queryVersion(query);
+        logDebug << "current db-version" << dbVersion.toString();
+        if(dbVersion != app::version()){
+            createOrUpDateDb(query, lock);
+        }
+    }
+    lock.unlock();
+
+    // Allow for delete queries with cascades
     query.exec("PRAGMA foreign_keys=ON");
 }
 

src/common/oscpp/cflock.cpp

@@ -4,6 +4,33 @@
 #include "cflock.h"
 #include "os.h"
 
+#ifndef NDEBUG
+
+static bool checkFdFlockFlags(int fd, int operation){
+    int flags = os::getFdStatusFlags(fd);
+
+    switch (operation) {
+    case LOCK_EX:
+        if(flags & O_WRONLY || flags & O_RDWR) {
+            return true;
+        }
+        std::cerr << "LOCK_EX: fd opened RDONLY\n";
+        break;
+    case LOCK_SH:
+        if(!(flags & O_WRONLY)) {
+            return true;
+        }
+        std::cerr << "LOCK_SH: fd opened WRONLY\n";
+        break;
+    default:
+        std::cerr << "Bad fd operation " << operation << "\n";
+        break;
+    }
+    return false;
+}
+
+#endif
+
 CFlock::CFlock(int fd) :
     m_fd(fd),
     m_isLocked(false)
@@ -23,12 +50,14 @@ CFlock::~CFlock()
 
 void CFlock::lockExclusive()
 {
+    assert(checkFdFlockFlags(m_fd, LOCK_EX));
     os::flock(m_fd, LOCK_EX);
     m_isLocked = true;
 }
 
 void CFlock::lockShared()
 {
+    assert(checkFdFlockFlags(m_fd, LOCK_SH));
     os::flock(m_fd, LOCK_SH);
     m_isLocked = true;
 }

src/common/oscpp/cflock.h

@@ -1,10 +1,16 @@
 #pragma once
 
 
-/// Warpper class for flock
+/// Wrapper class for flock.
+/// Due to NFS emulating flock as fcntl(2) byte-range locks
+/// the fd open mode must match the locking operations:
+/// In order to place a shared lock, fd must be open for reading,
+/// In order to place an exclusive lock, fd must be open for writing. To
+/// place both types of lock, open a file read-write.
 class CFlock
 {
 public:
+    /// fd should in general be opened read-write (see above)!
     CFlock(int fd);
     ~CFlock();