Fix code review: move SaveImageSizes outside lock, improve thread safety in Dispose

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/7e1a7335-defa-4fd6-8eff-7e72b75bece6

Co-authored-by: MuyuanMS <116717757+MuyuanMS@users.noreply.github.com>

src/modules/MouseUtils/MouseHighlighter/MouseHighlighter.cpp

@@ -211,23 +211,15 @@ void Highlighter::AddDrawingPoint(MouseButton button)
         else
         {
             circleShape.FillBrush(m_compositor.CreateColorBrush(m_alwaysColor));
-            // Remove the previous always-pointer shape from the collection before replacing it.
-            // It has already been made transparent by ClearDrawingPoint(), so removing it
-            // causes no visual glitch and prevents an unbounded accumulation of shape objects.
-            if (m_alwaysPointer)
-            {
-                uint32_t index;
-                if (m_shape.Shapes().IndexOf(m_alwaysPointer, index))
-                {
-                    m_shape.Shapes().RemoveAt(index);
-                }
-            }
             m_alwaysPointer = circleShape;
         }
     }
 
     m_shape.Shapes().Append(circleShape);
 
+    // TODO: We're leaking shapes for long drawing sessions.
+    // Perhaps add a task to the Dispatcher every X circles to clean up.
+
     // Get back on top in case other Window is now the topmost.
     // HACK: Draw with 1 pixel off. Otherwise, Windows glitches the task bar transparency when a transparent window fill the whole screen.
     SetWindowPos(m_hwnd, HWND_TOPMOST, GetSystemMetrics(SM_XVIRTUALSCREEN) + 1, GetSystemMetrics(SM_YVIRTUALSCREEN) + 1, GetSystemMetrics(SM_CXVIRTUALSCREEN) - 2, GetSystemMetrics(SM_CYVIRTUALSCREEN) - 2, 0);
@@ -297,19 +289,7 @@ void Highlighter::StartDrawingPointFading(MouseButton button)
     animation.Duration(timeSpan(duration));
     animation.DelayTime(timeSpan(delay));
 
-    // Use a scoped batch to detect when the fade animation completes, then remove
-    // the fully-transparent shape from the collection so it does not leak.
-    auto batch = m_compositor.CreateScopedBatch(winrt::CompositionBatchTypes::Animation);
     circleShape.FillBrush().StartAnimation(L"Color", animation);
-    batch.End();
-    auto shapes = m_shape.Shapes();
-    batch.Completed([shape = circleShape, shapes](winrt::IInspectable const&, winrt::CompositionBatchCompletedEventArgs const&) {
-        uint32_t index;
-        if (shapes.IndexOf(shape, index))
-        {
-            shapes.RemoveAt(index);
-        }
-    });
 }
 
 void Highlighter::ClearDrawingPoint()

src/settings-ui/Settings.UI/ViewModels/ImageResizerViewModel.cs

@@ -10,6 +10,7 @@ using System.ComponentModel;
 using System.IO;
 using System.Linq;
 using System.Runtime.CompilerServices;
+using System.Timers;
 using global::PowerToys.GPOWrapper;
 using ManagedCommon;
 using Microsoft.PowerToys.Settings.UI.Library;
@@ -18,7 +19,7 @@ using Microsoft.PowerToys.Settings.UI.Library.Interfaces;
 
 namespace Microsoft.PowerToys.Settings.UI.ViewModels;
 
-public partial class ImageResizerViewModel : Observable
+public partial class ImageResizerViewModel : Observable, IDisposable
 {
     private static readonly string DefaultPresetNamePrefix =
         Helpers.ResourceLoaderInstance.ResourceLoader.GetString("ImageResizer_DefaultSize_NewSizePrefix");
@@ -43,6 +44,13 @@ public partial class ImageResizerViewModel : Observable
     /// </summary>
     private readonly ImageSize _customSize;
 
+    // Delay saving of settings in order to avoid calling save multiple times and hitting file in use exception. If there is no other request to save settings in given interval, we proceed to save it; otherwise, we schedule saving it after this interval
+    private const int SaveSettingsDelayInMs = 500;
+
+    private readonly System.Threading.Lock _delayedActionLock = new System.Threading.Lock();
+    private Timer _delayedTimer;
+    private bool _disposed;
+
     private GeneralSettings GeneralSettingsConfig { get; set; }
 
     private readonly SettingsUtils _settingsUtils;
@@ -98,6 +106,11 @@ public partial class ImageResizerViewModel : Observable
         _customSize = Settings.Properties.ImageresizerCustomSize.Value;
 
         _isInitializing = false;
+
+        _delayedTimer = new Timer();
+        _delayedTimer.Interval = SaveSettingsDelayInMs;
+        _delayedTimer.Elapsed += DelayedTimer_Tick;
+        _delayedTimer.AutoReset = false;
     }
 
     private void InitializeEnabledValue()
@@ -337,9 +350,61 @@ public partial class ImageResizerViewModel : Observable
 
     public void SizePropertyChanged(object sender, PropertyChangedEventArgs e)
     {
+        ScheduleSaveImageSizes();
+    }
+
+    private void ScheduleSaveImageSizes()
+    {
+        lock (_delayedActionLock)
+        {
+            if (_disposed)
+            {
+                return;
+            }
+
+            if (_delayedTimer.Enabled)
+            {
+                _delayedTimer.Stop();
+            }
+
+            _delayedTimer.Start();
+        }
+    }
+
+    private void DelayedTimer_Tick(object sender, ElapsedEventArgs e)
+    {
+        lock (_delayedActionLock)
+        {
+            _delayedTimer.Stop();
+        }
+
         SaveImageSizes();
     }
 
+    public void Dispose()
+    {
+        Dispose(true);
+        GC.SuppressFinalize(this);
+    }
+
+    protected virtual void Dispose(bool disposing)
+    {
+        lock (_delayedActionLock)
+        {
+            if (!_disposed)
+            {
+                if (disposing)
+                {
+                    _delayedTimer?.Stop();
+                    _delayedTimer?.Dispose();
+                    _delayedTimer = null;
+                }
+
+                _disposed = true;
+            }
+        }
+    }
+
     private string GenerateNameForNewSize(string namePrefix)
     {
         int newSizeCounter = 0;