Merge pull request #288 from raisingthefloor/christopher/fix-highcont…

…rastreentrancy Add re-entrancy lock around 'set high contrast' code
raisingthefloor · Jul 21, 2021 · f503214 · f503214
2 parents 774a0ec + fca739a
commit f503214
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 25 deletions.
diff --git a/Morphic.Client/Bar/Data/Actions/Functions.cs b/Morphic.Client/Bar/Data/Actions/Functions.cs
@@ -24,7 +24,7 @@ namespace Morphic.Client.Bar.Data.Actions
     // ReSharper disable once UnusedType.Global - accessed via reflection.
     public class Functions
     {
-        private readonly static SemaphoreSlim _captureTextSemaphore = new SemaphoreSlim(1, 1);
+        private readonly static SemaphoreSlim s_captureTextSemaphore = new SemaphoreSlim(1, 1);
 
         [InternalFunction("snip")]
         public static async Task<IMorphicResult> ScreenSnipAsync(FunctionArgs args)
@@ -190,7 +190,7 @@ public static async Task<IMorphicResult> ReadAloudAsync(FunctionArgs args)
                         TextPatternRange[]? textRangeCollection = null;
                         //
                         // capture (or wait on) our "capture text" semaphore; we'll release this in the finally block
-                        await _captureTextSemaphore.WaitAsync();
+                        await s_captureTextSemaphore.WaitAsync();
                         //
                         try
                         {
@@ -220,7 +220,7 @@ public static async Task<IMorphicResult> ReadAloudAsync(FunctionArgs args)
                         }
                         finally
                         {
-                            _captureTextSemaphore.Release();
+                            s_captureTextSemaphore.Release();
                         }
                         //
                         // if we just captured a text range collection (i.e. were able to copy the current selection), convert that capture into a string now
@@ -260,7 +260,7 @@ public static async Task<IMorphicResult> ReadAloudAsync(FunctionArgs args)
                         if (captureTextViaAutomationSucceeded == false)
                         {
                             // capture (or wait on) our "capture text" semaphore; we'll release this in the finally block
-                            await _captureTextSemaphore.WaitAsync();
+                            await s_captureTextSemaphore.WaitAsync();
                             //
                             try
                             {
@@ -412,7 +412,7 @@ public static async Task<IMorphicResult> ReadAloudAsync(FunctionArgs args)
                             }
                             finally
                             {
-                                _captureTextSemaphore.Release();
+                                s_captureTextSemaphore.Release();
                             }
                         }
                     }

diff --git a/Morphic.Settings/SettingsHandlers/Theme/ThemeSettingsHandler.cs b/Morphic.Settings/SettingsHandlers/Theme/ThemeSettingsHandler.cs
@@ -11,13 +11,16 @@
     using Microsoft.Extensions.Logging;
     using Microsoft.Win32;
     using SolutionsRegistry;
+    using System.Threading;
 
     [SrService]
     public class ThemeSettingsHandler : FixedSettingsHandler
     {
         private readonly ILogger<ThemeSettingsHandler> logger;
         private readonly IRegistry registry;
 
+        private readonly static SemaphoreSlim s_setHighContrastSemaphore = new SemaphoreSlim(1, 1);
+
         public ThemeSettingsHandler(ILogger<ThemeSettingsHandler> logger, IRegistry registry)
         {
             this.logger = logger;
@@ -64,6 +67,7 @@ private void SaveCurrentTheme(string currentThemeFile, string saveAs)
                     "resources\\Themes\\aero.theme");
                 if (currentThemeFile != defaultTheme)
                 {
+                    // OBSERVATION: this code is re-entrant; also note that if the aero.theme file is corrupt...this might reenter infinitely until the stack was full
                     this.SaveCurrentTheme(defaultTheme, saveAs);
                 }
                 return;
@@ -129,32 +133,40 @@ private void SaveCurrentTheme(string currentThemeFile, string saveAs)
         }
 
         [Setter("highContrastEnabled")]
-        public Task<bool> SetHighContrast(Setting setting, object? newValue)
+        public async Task<bool> SetHighContrast(Setting setting, object? newValue)
         {
-            Spi.HighContrastOptions options = Spi.Instance.GetHighContrast();
-
-            bool enable = newValue as bool? == true;
-            bool currentlyEnabled = (options & Spi.HighContrastOptions.HCF_HIGHCONTRASTON) != 0;
-            if (!currentlyEnabled)
+            // capture (or wait on) our "set high contrast" semaphore; we'll release this in the finally block
+            await s_setHighContrastSemaphore.WaitAsync();
+            try
             {
-                // Save the current theme, otherwise windows will use the last saved theme when restoring the contrast
-                // mode.
-                ThemeSettingGroup settingGroup = (ThemeSettingGroup)setting.SettingGroup;
-                this.SaveCurrentTheme(settingGroup.CurrentTheme, settingGroup.SavedTheme);
-            }
+                Spi.HighContrastOptions options = Spi.Instance.GetHighContrast();
 
-            if (enable)
-            {
-                options |= Spi.HighContrastOptions.HCF_HIGHCONTRASTON;
+                bool enable = newValue as bool? == true;
+                bool currentlyEnabled = (options & Spi.HighContrastOptions.HCF_HIGHCONTRASTON) != 0;
+                if (!currentlyEnabled)
+                {
+                    // Save the current theme, otherwise windows will use the last saved theme when restoring the contrast
+                    // mode.
+                    ThemeSettingGroup settingGroup = (ThemeSettingGroup)setting.SettingGroup;
+                    this.SaveCurrentTheme(settingGroup.CurrentTheme, settingGroup.SavedTheme);
+                }
+
+                if (enable)
+                {
+                    options |= Spi.HighContrastOptions.HCF_HIGHCONTRASTON;
+                }
+                else
+                {
+                    options &= ~Spi.HighContrastOptions.HCF_HIGHCONTRASTON;
+                }
+
+                var setHighContrastSuccess = Spi.Instance.SetHighContrast(options);
+                return setHighContrastSuccess;
             }
-            else
+            finally
             {
-                options &= ~Spi.HighContrastOptions.HCF_HIGHCONTRASTON;
+                s_setHighContrastSemaphore.Release();
             }
-
-            Spi.Instance.SetHighContrast(options);
-
-            return Task.FromResult(true);
         }
 
         [Getter("highContrastEnabled")]