From 6e55cd1c3a87345f1dad1c79e618355ec394bda9 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@users.sourceforge.net>
Date: Thu, 18 Apr 2024 17:18:24 +0200
Subject: [PATCH] Base: Fix possible race condition when restarting the
 application

When restarting the application (e.g. after installing an addon) the application will be closed and a new instance will be launched. Now it can happen that the old instance is still busy writing the config files to disk while the new instance wants to read them in. At this time it's possible that a config file is in an invalid state so that the new instance will ignore it but then starts with a default configuration.
Later when closing the new instance the config files will be overwritten and destroy the user's original settings.

By using a lock file this race condition will be avoided. It uses a timeout of 1 second that should be enough for the old instance to write the files to disk.
---
 src/Base/Parameter.cpp       | 23 +++++++++++++++++++----
 tests/src/Base/Parameter.cpp | 20 ++++++++++++++++++++
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/src/Base/Parameter.cpp b/src/Base/Parameter.cpp
index fb09734944..69e4d32248 100644
--- a/src/Base/Parameter.cpp
+++ b/src/Base/Parameter.cpp
@@ -41,6 +41,10 @@
 #include <utility>
 #endif
 
+#include <QFileInfo>
+#include <QLockFile>
+#include <QDir>
+
 #ifdef FC_OS_LINUX
 #include <unistd.h>
 #endif
@@ -1719,6 +1723,17 @@ void ParameterManager::SaveDocument() const
     }
 }
 
+namespace
+{
+void waitForFileAccess(const Base::FileInfo& file)
+{
+    QFileInfo fi(QDir::tempPath(), QString::fromStdString(file.fileName() + ".lock"));
+    QLockFile lock(fi.absoluteFilePath());
+    const int waitOneSecond = 1000;
+    lock.tryLock(waitOneSecond);
+}
+}  // namespace
+
 //**************************************************************************
 // Document handling
 
@@ -1736,9 +1751,9 @@ bool ParameterManager::LoadOrCreateDocument(const char* sFileName)
 
 int ParameterManager::LoadDocument(const char* sFileName)
 {
-    Base::FileInfo file(sFileName);
-
     try {
+        Base::FileInfo file(sFileName);
+        waitForFileAccess(file);
 #if defined(FC_OS_WIN32)
         std::wstring name = file.toStdWString();
         // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
@@ -1828,9 +1843,9 @@ int ParameterManager::LoadDocument(const XERCES_CPP_NAMESPACE_QUALIFIER InputSou
 
 void ParameterManager::SaveDocument(const char* sFileName) const
 {
-    Base::FileInfo file(sFileName);
-
     try {
+        Base::FileInfo file(sFileName);
+        waitForFileAccess(file);
         //
         // Plug in a format target to receive the resultant
         // XML stream from the serializer.
diff --git a/tests/src/Base/Parameter.cpp b/tests/src/Base/Parameter.cpp
index a7aad26d20..a1f03b79e9 100644
--- a/tests/src/Base/Parameter.cpp
+++ b/tests/src/Base/Parameter.cpp
@@ -1,5 +1,6 @@
 #include "gtest/gtest.h"
 #include <boost/core/ignore_unused.hpp>
+#include <QLockFile>
 #include <Base/FileInfo.h>
 #include <Base/Parameter.h>
 
@@ -442,4 +443,23 @@ TEST_F(ParameterTest, TestObserverNoRef)
     EXPECT_EQ(obs.getCountNotifications(), 1);
 }
 
+TEST_F(ParameterTest, TestLockFile)
+{
+    std::string fn = getFileName();
+    fn.append(".lock");
+
+    QLockFile lockFile1(QString::fromStdString(fn));
+    EXPECT_TRUE(lockFile1.tryLock(100));
+    EXPECT_TRUE(lockFile1.isLocked());
+
+    QLockFile lockFile2(QString::fromStdString(fn));
+    EXPECT_FALSE(lockFile2.tryLock(100));
+    EXPECT_FALSE(lockFile2.isLocked());
+
+    lockFile1.unlock();
+    EXPECT_TRUE(lockFile2.lock());
+    EXPECT_FALSE(lockFile1.tryLock(500));
+    lockFile2.unlock();
+}
+
 // NOLINTEND(cppcoreguidelines-*,readability-*)