From 70520d44c3fa43565f37a4fa6dab30a95e5e209a Mon Sep 17 00:00:00 2001
From: Mathew Sutcliffe <oktal3700@gmail.com>
Date: Wed, 15 Mar 2017 18:36:45 +0000
Subject: [PATCH] refs #884 Protect filesystem race in settings using
 QLockFile.

---
 src/blackcore/application.cpp   |  3 ---
 src/blackmisc/settingscache.cpp | 40 +++++++++++++++++++++++++++++++++
 src/blackmisc/settingscache.h   |  2 ++
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/src/blackcore/application.cpp b/src/blackcore/application.cpp
index da8c64868..a17c86ad9 100644
--- a/src/blackcore/application.cpp
+++ b/src/blackcore/application.cpp
@@ -511,9 +511,6 @@ namespace BlackCore
         {
             return this->getIContextApplication()->saveSettingsByKey(keys);
         }
-        //! \todo If contexts have shut down then we already missed the opportunity to save settings.
-        //! Saving without contexts is not safe.
-        BLACK_VERIFY(false);
         return CSettingsCache::instance()->saveToStore(keys);
     }
 
diff --git a/src/blackmisc/settingscache.cpp b/src/blackmisc/settingscache.cpp
index 9c7f9f46b..7f4f10e84 100644
--- a/src/blackmisc/settingscache.cpp
+++ b/src/blackmisc/settingscache.cpp
@@ -30,13 +30,41 @@ namespace BlackMisc
         return dir;
     }
 
+    const QString &CSettingsCache::lockFileName()
+    {
+        static const QString file = CFileUtils::appendFilePaths(persistentStore(), ".lock");
+        return file;
+    }
+
+    CStatusMessage CSettingsCache::lockFile(QLockFile &lock)
+    {
+        Q_ASSERT(!lock.isLocked());
+        if (!QDir::root().mkpath(persistentStore()))
+        {
+            return CStatusMessage(this).error("Failed to create %1") << persistentStore();
+        }
+        if (!lock.lock())
+        {
+            return CStatusMessage(this).error("Failed to lock %1: %2") << lockFileName() << CFileUtils::lockFileError(lock);
+        }
+        return {};
+    }
+
     BlackMisc::CStatusMessage CSettingsCache::saveToStore(const QString &keyPrefix)
     {
+        QLockFile lock(lockFileName());
+        const CStatusMessage lockStatus = lockFile(lock);
+        if (lockStatus.isFailure()) { return lockStatus; }
+
         return saveToFiles(persistentStore(), keyPrefix);
     }
 
     BlackMisc::CStatusMessage CSettingsCache::saveToStore(const QStringList &keys)
     {
+        QLockFile lock(lockFileName());
+        const CStatusMessage lockStatus = lockFile(lock);
+        if (lockStatus.isFailure()) { return lockStatus; }
+
         return saveToFiles(persistentStore(), keys);
     }
 
@@ -47,12 +75,24 @@ namespace BlackMisc
 
     void CSettingsCache::saveToStoreByPacket(const CValueCachePacket &values)
     {
+        QLockFile lock(lockFileName());
+        const CStatusMessage lockStatus = lockFile(lock);
+        if (lockStatus.isFailure())
+        {
+            CLogMessage::preformatted(lockStatus);
+            return;
+        }
+
         CStatusMessage status = saveToFiles(persistentStore(), values.toVariantMap());
         CLogMessage::preformatted(status);
     }
 
     BlackMisc::CStatusMessage CSettingsCache::loadFromStore()
     {
+        QLockFile lock(lockFileName());
+        const CStatusMessage lockStatus = lockFile(lock);
+        if (lockStatus.isFailure()) { return lockStatus; }
+
         return loadFromFiles(persistentStore());
     }
 
diff --git a/src/blackmisc/settingscache.h b/src/blackmisc/settingscache.h
index dacc52885..56d679b50 100644
--- a/src/blackmisc/settingscache.h
+++ b/src/blackmisc/settingscache.h
@@ -62,6 +62,8 @@ namespace BlackMisc
     private:
         CSettingsCache();
         void saveToStoreByPacket(const CValueCachePacket &values);
+        static const QString &lockFileName();
+        CStatusMessage lockFile(QLockFile &);
     };
 
     /*!