PowerToys Run cache issue (#4472)

* Clean termination of powertoys process. * Fixed issue with run not responding to WM_CLOSE * Fixed serialization error in pinyin and image cache * Fixed merge conflict * Fixed nit wrt to master * Fixed undeterministic behaviour of Environment.Exit function * Update timing for terminate process
microsoft · Jun 25, 2020 · 92fa8b7 · 92fa8b7
1 parent aad2e80
commit 92fa8b7
Show file tree

Hide file tree

Showing 9 changed files with 112 additions and 36 deletions.
diff --git a/src/common/ManagedCommon/RunnerHelper.cs b/src/common/ManagedCommon/RunnerHelper.cs
@@ -11,7 +11,7 @@ namespace ManagedCommon
 {
     public static class RunnerHelper
     {
-        public static void WaitForPowerToysRunner(int powerToysPID)
+        public static void WaitForPowerToysRunner(int powerToysPID, Action act)
         {
             var stackTrace = new StackTrace();
             var assembly = Assembly.GetCallingAssembly().GetName();
@@ -27,7 +27,7 @@ public static void WaitForPowerToysRunner(int powerToysPID)
                 if (NativeMethods.WaitForSingleObject(powerToysProcHandle, INFINITE) == WAIT_OBJECT_0)
                 {
                     PowerToysTelemetry.Log.WriteEvent(new DebugEvent() { Message = $"[{assembly}][{callingMethod}]WaitForPowerToysRunner Event Notified powerToysPID={powerToysPID}" });
-                    Environment.Exit(0);
+                    act.Invoke();
                 }
             });
         }

diff --git a/src/core/Microsoft.PowerToys.Settings.UI.Runner/Program.cs b/src/core/Microsoft.PowerToys.Settings.UI.Runner/Program.cs
@@ -58,7 +58,9 @@ public static void Main(string[] args)
                         IsUserAnAdmin = false;
                     }
 
-                    RunnerHelper.WaitForPowerToysRunner(PowerToysPID);
+                    RunnerHelper.WaitForPowerToysRunner(PowerToysPID, () => {
+                        Environment.Exit(0);
+                    });
 
                     ipcmanager = new TwoWayPipeMessageIPCManaged(args[1], args[0], null);
                     ipcmanager.Start();

diff --git a/src/modules/fancyzones/editor/FancyZonesEditor/App.xaml.cs b/src/modules/fancyzones/editor/FancyZonesEditor/App.xaml.cs
@@ -25,7 +25,9 @@ public App()
 
         private void OnStartup(object sender, StartupEventArgs e)
         {
-            RunnerHelper.WaitForPowerToysRunner(Settings.PowerToysPID);
+            RunnerHelper.WaitForPowerToysRunner(Settings.PowerToysPID, () => {
+                Environment.Exit(0);
+            });
 
             LayoutModel foundModel = null;
 

diff --git a/src/modules/launcher/Microsoft.Launcher/dllmain.cpp b/src/modules/launcher/Microsoft.Launcher/dllmain.cpp
@@ -45,6 +45,10 @@ class Microsoft_Launcher : public PowertoyModuleIface {
   //contains the name of the powerToys
   std::wstring app_name;
 
+  // Time to wait for process to close after sending WM_CLOSE signal
+  static const int MAX_WAIT_MILLISEC = 10000;
+
+
 public:
   // Constructor
   Microsoft_Launcher() {
@@ -55,7 +59,7 @@ class Microsoft_Launcher : public PowertoyModuleIface {
   ~Microsoft_Launcher() {
       if (m_enabled)
       {
-          TerminateProcess(m_hProcess, 1);
+          terminateProcess();
       }
       m_enabled = false;
   }
@@ -201,8 +205,8 @@ class Microsoft_Launcher : public PowertoyModuleIface {
   virtual void disable()
   {
       if (m_enabled)
-      {
-          TerminateProcess(m_hProcess, 1);
+      {      
+          terminateProcess();
       }
 
       m_enabled = false;
@@ -229,6 +233,28 @@ class Microsoft_Launcher : public PowertoyModuleIface {
     return 0;
   }
 
+  // Callback to send WM_CLOSE signal to each top level window.
+  static BOOL CALLBACK requestMainWindowClose(HWND nextWindow, LPARAM closePid) {
+      DWORD windowPid;
+      GetWindowThreadProcessId(nextWindow, &windowPid);
+
+      if (windowPid == (DWORD)closePid)
+          ::PostMessage(nextWindow, WM_CLOSE, 0, 0);
+
+      return true;
+  }
+
+  // Terminate process by sending WM_CLOSE signal and if it fails, force terminate.
+  void terminateProcess() {
+      DWORD processID = GetProcessId(m_hProcess);
+      EnumWindows(&requestMainWindowClose, processID);
+      const DWORD result = WaitForSingleObject(m_hProcess, MAX_WAIT_MILLISEC);
+      if (result == WAIT_TIMEOUT)
+      {
+          TerminateProcess(m_hProcess, 1);
+      }
+  }
+
   /* Register helper class to handle system menu items related actions. */
   virtual void register_system_menu_helper(PowertoySystemMenuIface* helper) {}
   /* Handle action on system menu item. */

diff --git a/src/modules/launcher/PowerLauncher/App.xaml.cs b/src/modules/launcher/PowerLauncher/App.xaml.cs
@@ -26,7 +26,7 @@ public partial class App : IDisposable, ISingleInstanceApp
     {
         public static PublicAPIInstance API { get; private set; }
         private const string Unique = "PowerLauncher_Unique_Application_Mutex";
-        private static bool _disposed;
+        private static bool _disposed = false;
         private static int _powerToysPid;
         private Settings _settings;
         private MainViewModel _mainVM;
@@ -56,13 +56,16 @@ public static void Main(string[] args)
 
         private void OnStartup(object sender, StartupEventArgs e)
         {
-            RunnerHelper.WaitForPowerToysRunner(_powerToysPid);
+            RunnerHelper.WaitForPowerToysRunner(_powerToysPid, () => {
+                Dispose();
+                Environment.Exit(0);
+            });
 
             var bootTime = new System.Diagnostics.Stopwatch();
             bootTime.Start();
             Stopwatch.Normal("|App.OnStartup|Startup cost", () =>
             {
-                Log.Info("|App.OnStartup|Begin Wox startup ----------------------------------------------------");
+                Log.Info("|App.OnStartup|Begin PowerToys Run startup ----------------------------------------------------");
                 Log.Info($"|App.OnStartup|Runtime info:{ErrorReporting.RuntimeInfo()}");
                 RegisterAppDomainExceptions();
                 RegisterDispatcherUnhandledException();
@@ -102,7 +105,7 @@ private void OnStartup(object sender, StartupEventArgs e)
                 
                 _mainVM.MainWindowVisibility = Visibility.Visible;
                 _mainVM.ColdStartFix();
-                Log.Info("|App.OnStartup|End Wox startup ----------------------------------------------------  ");
+                Log.Info("|App.OnStartup|End PowerToys Run startup ----------------------------------------------------  ");
 
                 bootTime.Stop();
 
@@ -150,16 +153,22 @@ protected virtual void Dispose(bool disposing)
         {
             if (!_disposed)
             {
-                if (disposing)
+                Stopwatch.Normal("|App.OnExit|Exit cost", () =>
                 {
-                    _mainWindow.Dispose();
-                    API.SaveAppAllSettings();
+                    Log.Info("|App.OnExit| Start PowerToys Run Exit----------------------------------------------------  ");
+                    if (disposing)
+                    {
+                        _mainWindow.Dispose();
+                        API.SaveAppAllSettings();
+                        _mainVM.Dispose();
+                        _disposed = true;
+                    }
+
+                    // TODO: free unmanaged resources (unmanaged objects) and override finalizer
+                    // TODO: set large fields to null
                     _disposed = true;
-                }
-
-                // TODO: free unmanaged resources (unmanaged objects) and override finalizer
-                // TODO: set large fields to null
-                _disposed = true;
+                    Log.Info("|App.OnExit| End PowerToys Run Exit ----------------------------------------------------  ");
+                });
             }
         }
 

diff --git a/src/modules/launcher/Wox.Infrastructure/Alphabet.cs b/src/modules/launcher/Wox.Infrastructure/Alphabet.cs
@@ -21,7 +21,7 @@ public class Alphabet : IAlphabet
     {
         private readonly HanyuPinyinOutputFormat Format = new HanyuPinyinOutputFormat();
         private ConcurrentDictionary<string, string[][]> PinyinCache;
-        private BinaryStorage<ConcurrentDictionary<string, string[][]>> _pinyinStorage;
+        private BinaryStorage<Dictionary<string, string[][]>> _pinyinStorage;
         private Settings _settings;
 
         public void Initialize([NotNull] Settings settings)
@@ -36,8 +36,8 @@ private void InitializePinyinHelpers()
 
             Stopwatch.Normal("|Wox.Infrastructure.Alphabet.Initialize|Preload pinyin cache", () =>
             {
-                _pinyinStorage = new BinaryStorage<ConcurrentDictionary<string, string[][]>>("Pinyin");
-                PinyinCache = _pinyinStorage.TryLoad(new ConcurrentDictionary<string, string[][]>());
+                _pinyinStorage = new BinaryStorage<Dictionary<string, string[][]>>("Pinyin");
+                SetPinyinCacheAsDictionary(_pinyinStorage.TryLoad(new Dictionary<string, string[][]>()));
 
                 // force pinyin library static constructor initialize
                 PinyinHelper.toHanyuPinyinStringArray('T', Format);
@@ -79,7 +79,7 @@ public void Save()
             {
                 return; 
             }
-            _pinyinStorage.Save(PinyinCache);
+            _pinyinStorage.Save(GetPinyinCacheAsDictionary());
         }
 
         private static string[] EmptyStringArray = new string[0];
@@ -185,5 +185,15 @@ private string[] Combination(string[] array1, string[] array2)
             ).ToArray();
             return combination;
         }
+
+        private Dictionary<string, string[][]> GetPinyinCacheAsDictionary()
+        {
+            return new Dictionary<string, string[][]>(PinyinCache);
+        }
+
+        private void SetPinyinCacheAsDictionary(Dictionary<string, string[][]> usage)
+        {
+            PinyinCache = new ConcurrentDictionary<string, string[][]>(usage);
+        }
     }
 }
diff --git a/src/modules/launcher/Wox.Infrastructure/Image/ImageCache.cs b/src/modules/launcher/Wox.Infrastructure/Image/ImageCache.cs
@@ -52,6 +52,16 @@ public int UniqueImagesInCache()
         {
             return _data.Values.Distinct().Count();
         }
+
+        public Dictionary<string, int> GetUsageAsDictionary()
+        {
+            return new Dictionary<string, int>(Usage);
+        }
+
+        public void SetUsageAsDictionary(Dictionary<string, int> usage)
+        {
+            Usage = new ConcurrentDictionary<string, int>(usage);
+        }
     }
 
 }
diff --git a/src/modules/launcher/Wox.Infrastructure/Image/ImageLoader.cs b/src/modules/launcher/Wox.Infrastructure/Image/ImageLoader.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Concurrent;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
@@ -13,7 +14,7 @@ namespace Wox.Infrastructure.Image
     public static class ImageLoader
     {
         private static readonly ImageCache ImageCache = new ImageCache();
-        private static BinaryStorage<ConcurrentDictionary<string, int>> _storage;
+        private static BinaryStorage<Dictionary<string, int>> _storage;
         private static readonly ConcurrentDictionary<string, string> GuidToKey = new ConcurrentDictionary<string, string>();
         private static IImageHashGenerator _hashGenerator;
 
@@ -32,9 +33,9 @@ public static class ImageLoader
 
         public static void Initialize()
         {
-            _storage = new BinaryStorage<ConcurrentDictionary<string, int>>("Image");
+            _storage = new BinaryStorage<Dictionary<string, int>>("Image");
             _hashGenerator = new ImageHashGenerator();
-            ImageCache.Usage = _storage.TryLoad(new ConcurrentDictionary<string, int>());
+            ImageCache.SetUsageAsDictionary(_storage.TryLoad(new Dictionary<string, int>()));
 
             foreach (var icon in new[] { Constant.DefaultIcon, Constant.ErrorIcon })
             {
@@ -58,7 +59,7 @@ public static void Initialize()
         public static void Save()
         {
             ImageCache.Cleanup();
-            _storage.Save(ImageCache.Usage);
+            _storage.Save(ImageCache.GetUsageAsDictionary());
         }
 
         private class ImageResult

diff --git a/src/modules/launcher/Wox/ViewModel/MainViewModel.cs b/src/modules/launcher/Wox/ViewModel/MainViewModel.cs
@@ -22,12 +22,13 @@
 
 namespace Wox.ViewModel
 {
-    public class MainViewModel : BaseModel, ISavable
+    public class MainViewModel : BaseModel, ISavable, IDisposable
     {
         #region Private Fields
 
         private bool _isQueryRunning;
         private Query _lastQuery;
+        private static bool _disposed;
         private string _queryTextBeforeLeaveResults;
 
         private readonly WoxJsonStorage<History> _historyItemsStorage;
@@ -55,6 +56,7 @@ public MainViewModel(Settings settings)
             _saved = false;
             _queryTextBeforeLeaveResults = "";
             _lastQuery = new Query();
+            _disposed = false;
 
             _settings = settings;
 
@@ -112,14 +114,6 @@ private void RegisterResultsUpdatedEvent()
             }
         }
 
-        ~MainViewModel()
-        {
-            if (_hotkeyHandle != 0)
-            {
-                _hotkeyManager.UnregisterHotkey(_hotkeyHandle);
-            }
-        }
-
         private void InitializeKeyCommands()
         {
             IgnoreCommand = new RelayCommand(_ => {});
@@ -715,6 +709,28 @@ public void HandleContextMenu(Key AcceleratorKey, ModifierKeys AcceleratorModifi
             }
         }
 
+        protected virtual void Dispose(bool disposing)
+        {
+            if (!_disposed)
+            {
+                if (disposing)
+                {
+                    if (_hotkeyHandle != 0)
+                    {
+                        _hotkeyManager.UnregisterHotkey(_hotkeyHandle);
+                    }
+                    _hotkeyManager.Dispose();
+                    _disposed = true;
+                }   
+            }         
+        }
+
+        public void Dispose()
+        {
+            Dispose(disposing: true);
+            GC.SuppressFinalize(this);
+        }
+
         #endregion
     }
 }