ecere/GlobalAppSettings; ide/IDESettings: Fixed global settings sharing bugs

- Fixed wipe out of global settings caused by locking only after wiping
- Fixed wipe out protection MessageBox popping up when it should not (Checking the file without a lock)
- Added missing isModal = true to MessageBox pop up

diff --git a/ecere/src/sys/GlobalAppSettings.ec b/ecere/src/sys/GlobalAppSettings.ec
index 405ed71..0e3fd9d 100644 (file)
--- a/ecere/src/sys/GlobalAppSettings.ec
+++ b/ecere/src/sys/GlobalAppSettings.ec
@@ -140,6 +140,7 @@ private:
       }
    };
    File f;
+   bool locked;
 
    subclass(GlobalSettingsDriver) driverClass;
 
@@ -327,24 +328,34 @@ private:
          delete settingsFilePath;  // the case when we're doing a load when the config file is no longer available
    }                               // and we want to re-try all possible config locations.
 
-   bool FileOpenTryWrite(bool shouldDelete)
+   bool FileOpenTryWrite(bool shouldDelete, bool * locked)
    {
-      if(driverClass)
-         f = FileOpen(settingsFilePath, write);
-      else
+      *locked = false;
+      f = FileOpen(settingsFilePath, readWrite);
+      if(!f)
       {
-         if(!(f = FileOpen(settingsFilePath, readWrite)))
+         f = FileOpen(settingsFilePath, writeRead);
+         if(!driverClass)
          {
-            f = FileOpen(settingsFilePath, writeRead);
             delete f;
             f = FileOpen(settingsFilePath, readWrite);
          }
       }
-      /*    NOTE: This used to handle chaining to write to a where the user has permission after loading from a global settings file.
-                  This was broken a while back, and is now working through checking 'f' for non-null if 'globalPath' is true.
-                  This new way has the advantage of avoiding using different alternate user profile settings files if the first one fails for whatever reason.
-      */
-      if(!f && shouldDelete)       // This delete will cover both trying the next possible config location and
+      if(f)
+      {
+         // Don't wait for a lock, first one to lock gets to write, other will likely loose changes on a reload.
+         if(f.Lock(exclusive, 0, 0, false))
+         {
+            *locked = true;
+            if(driverClass)
+            {
+               f.Truncate(0);
+               f.Seek(0, start);
+            }
+         }
+      }
+
+      if(shouldDelete)       // This delete will cover both trying the next possible config location and
          delete settingsFilePath;  // allow trying to save to a location where user has permission.
       return f != null;
    }
@@ -352,7 +363,7 @@ private:
 public:
    virtual void OnAskReloadSettings();
 
-   virtual SettingsIOResult Load()
+   bool OpenAndLock(FileSize * fileSize)
    {
       SettingsIOResult result = fileNotFound;
       if(!f)
@@ -405,11 +416,12 @@ public:
 #endif
             }
          }
-
-         if(f)
+      }
+      if(f)
+      {
+         int c;
+         if(!locked)
          {
-            int c;
-            bool locked;
             // At some point wait was true, it was changed to false and now we do retries.
             // Could it be because the wait to true was causing blocking behaviors?
             //if(f && f.Lock(shared, 0, 0, true)) <- I think the wait to true is bad, it wrote blanked out global settings.
@@ -417,15 +429,26 @@ public:
             {
                ecere::sys::Sleep(0.01);
             }
-            if(locked)
-            {
-               if(driverClass)
-                  result = driverClass.Load(f, this);
-               else
-                  result = success;
-            }
          }
 
+         if(locked && fileSize)
+            *fileSize = f.GetSize();
+      }
+      return f && locked;
+   }
+
+   virtual SettingsIOResult Load()
+   {
+      SettingsIOResult result = fileNotFound;
+      if(!f || !locked)
+         OpenAndLock(null);
+
+      if(f && locked)
+      {
+         if(driverClass)
+            result = driverClass.Load(f, this);
+         else
+            result = success;
       }
       return result;
    }
@@ -436,19 +459,20 @@ public:
       if(!f)
       {
          char filePath[MAX_LOCATION];
+         locked = false;
 
          settingsMonitor.StopMonitoring();
 
          if(settingsFilePath)
             // Don't auto delete settingsFilePath because only want to try another path if we were using a global path
-            FileOpenTryWrite(false);
+            FileOpenTryWrite(false, &locked);
 
          if((!settingsFilePath || (!f && globalPath)) && settingsName && settingsName[0])
          {
             delete settingsFilePath;
 
             if(!f && (settingsFilePath = PrepareSpecifiedLocationPath()))
-               FileOpenTryWrite(true);
+               FileOpenTryWrite(true, &locked);
             if(!f && (!settingsLocation || allowDefaultLocations))
             {
                globalPath = true;
@@ -458,10 +482,10 @@ public:
                //   FileOpenTryWrite(true);
 #if defined(__WIN32__)
                if(!f && (settingsFilePath = PrepareAllUsersPath()))
-                  FileOpenTryWrite(true);
+                  FileOpenTryWrite(true, &locked);
 #else
                if(!f && (settingsFilePath = PrepareEtcPath()))
-                  FileOpenTryWrite(true);
+                  FileOpenTryWrite(true, &locked);
 #endif
                if(!f && allUsers)
                {
@@ -473,27 +497,26 @@ public:
                      false
 #endif
                      )))
-                     FileOpenTryWrite(true);
+                     FileOpenTryWrite(true, &locked);
                }
 #if defined(__WIN32__)
                if(!f && !allUsers)
                {
                   globalPath = false;
                   if(!f && (settingsFilePath = PrepareUserProfilePath()))
-                     FileOpenTryWrite(true);
+                     FileOpenTryWrite(true, &locked);
                   if(!f && (settingsFilePath = PrepareHomeDrivePath()))
-                     FileOpenTryWrite(true);
+                     FileOpenTryWrite(true, &locked);
                }
                if(!f && (settingsFilePath = PrepareSystemPath()))
                {
                   globalPath = true;
-                  FileOpenTryWrite(true);
+                  FileOpenTryWrite(true, &locked);
                }
 #endif
             }
          }
-         // Don't wait for a lock, first one to lock gets to write, other will likely loose changes on a reload.
-         if(f && f.Lock(exclusive, 0, 0, false))
+         if(f && locked)
          {
             if(driverClass)
                result = driverClass.Save(f, this);
@@ -510,6 +533,7 @@ public:
       {
          settingsMonitor.StopMonitoring();
          f.Unlock(0,0,true);
+         locked = false;
          delete f;
       }
    }

diff --git a/ide/src/IDESettings.ec b/ide/src/IDESettings.ec
index 296ddc3..b268f5a 100644 (file)
--- a/ide/src/IDESettings.ec
+++ b/ide/src/IDESettings.ec
@@ -286,30 +286,26 @@ private:
 
    void OnAskReloadSettings()
    {
-      /*if(MessageBox { type = YesNo, master = this,
-            text = "Global Settings Modified Externally",
-            contents = "The global settings were modified by another instance.\n"
-            "Would you like to reload them?" }.Modal() == Yes)*/
+      FileSize newSettingsFileSize;
+
+      if(OpenAndLock(&newSettingsFileSize))
       {
-         double o, n;
-         FileSize newSettingsFileSize;
-         FileGetSize(settingsFilePath, &newSettingsFileSize);
-         o = settingsFileSize;
-         n = newSettingsFileSize;
-         if(o - n < 2048)
+         if((double)settingsFileSize - (double)newSettingsFileSize < 2048)
             Load();
          else
          {
             GuiApplication app = ((GuiApplication)__thisModule.application);
             Window w;
             for(w = app.desktop.firstChild; w && (!w.created || !w.visible); w = w.next);
-            MessageBox { master = w, type = ok,
+
+            CloseAndMonitor();
+
+            MessageBox { master = w, type = ok, isModal = true,
                   text = "Global Settings Modified Externally",
                   contents = "The global settings were modified by another process and a drastic shrinking of the settings file was detected.\n"
                   "The new settings will not be loaded to prevent loss of your ide settings.\n"
                   "Please check your settings file and make sure to save this IDE's global settings if your settings file has been compromised."
                   }.Create();
-            //Save();
          }
       }
    }