From 9bb20378ee30eb0de227701a0849e0559947486b Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 29 Jun 2021 20:10:24 -0700 Subject: [PATCH 01/18] Change how background threads operate This should reduce CPU usage. --- src/modules/awake/Awake/Core/APIHelper.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/modules/awake/Awake/Core/APIHelper.cs b/src/modules/awake/Awake/Core/APIHelper.cs index ffa242a864b..da128f639b1 100644 --- a/src/modules/awake/Awake/Core/APIHelper.cs +++ b/src/modules/awake/Awake/Core/APIHelper.cs @@ -184,13 +184,10 @@ private static bool RunIndefiniteLoop(bool keepDisplayOn = false) if (success) { _log.Info($"Initiated indefinite keep awake in background thread: {GetCurrentThreadId()}. Screen on: {keepDisplayOn}"); - while (true) - { - if (_threadToken.IsCancellationRequested) - { - _threadToken.ThrowIfCancellationRequested(); - } - } + + WaitHandle.WaitAny(new[] { _threadToken.WaitHandle }); + + return success; } else { From 05523644d7f36c28714d675da83ad9c8ce7804ba Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 29 Jun 2021 20:12:39 -0700 Subject: [PATCH 02/18] Well there is just no need for the console here --- src/modules/awake/Awake/Program.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index 9087ad848ee..81d793f1b11 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -27,7 +27,7 @@ namespace Awake internal class Program { private static Mutex? _mutex = null; - private const string AppName = "Awake"; + private const string AppName = "PowerToys Awake"; private static FileSystemWatcher? _watcher = null; private static SettingsUtils? _settingsUtils = null; @@ -127,7 +127,6 @@ private static int Main(string[] args) private static void ForceExit(string message, int exitCode) { _log.Info(message); - Console.ReadKey(); Environment.Exit(exitCode); } From 0946f061a1f8a502c0f5fba77ed7d8a5e0532c3a Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 29 Jun 2021 20:27:43 -0700 Subject: [PATCH 03/18] Need to keep the full name sanitized --- src/modules/awake/Awake/Program.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index 81d793f1b11..1cb660356c5 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -27,7 +27,8 @@ namespace Awake internal class Program { private static Mutex? _mutex = null; - private const string AppName = "PowerToys Awake"; + private const string AppName = "Awake"; + private const string FullAppName = "PowerToys " + AppName; private static FileSystemWatcher? _watcher = null; private static SettingsUtils? _settingsUtils = null; @@ -179,7 +180,7 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, .Select(e => e.EventArgs) .Subscribe(HandleAwakeConfigChange); - TrayHelper.SetTray(AppName, new AwakeSettings()); + TrayHelper.SetTray(FullAppName, new AwakeSettings()); // Initially the file might not be updated, so we need to start processing // settings right away. From 1d612b64c5d0d34610975a7477efa153c6e8edbb Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Sun, 4 Jul 2021 18:06:15 -0700 Subject: [PATCH 04/18] Update the changes to console handling --- src/modules/awake/Awake/Core/APIHelper.cs | 31 +++++++++++++++++++++- src/modules/awake/Awake/Core/TrayHelper.cs | 25 ++++++++++------- src/modules/awake/Awake/Program.cs | 28 ++++++++++++++++--- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/modules/awake/Awake/Core/APIHelper.cs b/src/modules/awake/Awake/Core/APIHelper.cs index da128f639b1..85c02352808 100644 --- a/src/modules/awake/Awake/Core/APIHelper.cs +++ b/src/modules/awake/Awake/Core/APIHelper.cs @@ -21,6 +21,18 @@ public enum EXECUTION_STATE : uint ES_SYSTEM_REQUIRED = 0x00000001, } + // See: https://docs.microsoft.com/windows/console/handlerroutine + public enum ControlType + { + CTRL_C_EVENT = 0, + CTRL_BREAK_EVENT = 1, + CTRL_CLOSE_EVENT = 2, + CTRL_LOGOFF_EVENT = 5, + CTRL_SHUTDOWN_EVENT = 6, + } + + public delegate bool ConsoleEventHandler(ControlType ctrlType); + /// /// Helper class that allows talking to Win32 APIs without having to rely on PInvoke in other parts /// of the codebase. @@ -38,6 +50,9 @@ public class APIHelper private static Task? _runnerThread; + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool SetConsoleCtrlHandler(ConsoleEventHandler handler, bool add); + [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] private static extern EXECUTION_STATE SetThreadExecutionState(EXECUTION_STATE esFlags); @@ -45,6 +60,10 @@ public class APIHelper [return: MarshalAs(UnmanagedType.Bool)] private static extern bool AllocConsole(); + [DllImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool FreeConsole(); + [DllImport("kernel32.dll", SetLastError = true)] private static extern bool SetStdHandle(int nStdHandle, IntPtr hHandle); @@ -67,6 +86,16 @@ static APIHelper() _tokenSource = new CancellationTokenSource(); } + public static void SetConsoleControlHandler(ConsoleEventHandler handler, bool addHandler) + { + SetConsoleCtrlHandler(handler, addHandler); + } + + public static void DeallocateConsole() + { + FreeConsole(); + } + public static void AllocateConsole() { _log.Debug("Bootstrapping the console allocation routine."); @@ -139,7 +168,7 @@ public static void SetNoKeepAwake() } catch (OperationCanceledException) { - _log.Info("Confirmed background thread cancellation when setting passive keep awake."); + _log.Info("Confirmed background thread cancellation when disabling explicit keep awake."); } } diff --git a/src/modules/awake/Awake/Core/TrayHelper.cs b/src/modules/awake/Awake/Core/TrayHelper.cs index ec44ae49eac..d8b814c819c 100644 --- a/src/modules/awake/Awake/Core/TrayHelper.cs +++ b/src/modules/awake/Awake/Core/TrayHelper.cs @@ -6,6 +6,7 @@ using System.Drawing; using System.IO; using System.Text.Json; +using System.Threading.Tasks; using System.Windows.Forms; using Microsoft.PowerToys.Settings.UI.Library; @@ -32,16 +33,22 @@ static TrayHelper() public static void InitializeTray(string text, Icon icon, ContextMenuStrip? contextMenu = null) { - System.Threading.Tasks.Task.Factory.StartNew( + Task.Factory.StartNew( (tray) => - { - ((NotifyIcon?)tray).Text = text; - ((NotifyIcon?)tray).Icon = icon; - ((NotifyIcon?)tray).ContextMenuStrip = contextMenu; - ((NotifyIcon?)tray).Visible = true; + { + ((NotifyIcon?)tray).Text = text; + ((NotifyIcon?)tray).Icon = icon; + ((NotifyIcon?)tray).ContextMenuStrip = contextMenu; + ((NotifyIcon?)tray).Visible = true; - Application.Run(); - }, TrayIcon); + Application.Run(); + }, TrayIcon); + } + + public static void ClearTray() + { + TrayIcon.Icon = null; + TrayIcon.Dispose(); } internal static void SetTray(string text, AwakeSettings settings) @@ -61,7 +68,7 @@ private static Action ExitCallback() { return () => { - Environment.Exit(0); + Environment.Exit(Environment.ExitCode); }; } diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index 1cb660356c5..8d6795cb179 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -36,6 +36,12 @@ internal class Program private static Logger? _log; +#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. + private static ConsoleEventHandler _handler; +#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. + + private static ManualResetEvent _exitSignal = new ManualResetEvent(false); + private static int Main(string[] args) { bool instantiated; @@ -125,9 +131,22 @@ private static int Main(string[] args) return rootCommand.InvokeAsync(args).Result; } + private static bool ExitHandler(ControlType ctrlType) + { + _log.Info($"Exited through handler with control type: {ctrlType}"); + + ForceExit("Exiting from the internal termination handler.", Environment.ExitCode); + + return false; + } + private static void ForceExit(string message, int exitCode) { _log.Info(message); + + APIHelper.SetNoKeepAwake(); + TrayHelper.ClearTray(); + Environment.Exit(exitCode); } @@ -137,6 +156,9 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, { _log.Info("No PID specified. Allocating console..."); APIHelper.AllocateConsole(); + + _handler += new ConsoleEventHandler(ExitHandler); + APIHelper.SetConsoleControlHandler(_handler, true); } _log.Info($"The value for --use-pt-config is: {usePtConfig}"); @@ -207,17 +229,15 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, } } - var exitSignal = new ManualResetEvent(false); if (pid != 0) { RunnerHelper.WaitForPowerToysRunner(pid, () => { - exitSignal.Set(); - Environment.Exit(0); + ForceExit("Terminating from PowerToys binding hook.", 0); }); } - exitSignal.WaitOne(); + _exitSignal.WaitOne(); } private static void SetupIndefiniteKeepAwake(bool displayOn) From 1a19a5c2fc95bfba38db298a435bcd6e24225105 Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Sun, 4 Jul 2021 18:15:24 -0700 Subject: [PATCH 05/18] Updating how we handle constants --- src/modules/awake/Awake/Core/Constants.cs | 12 ++++++++++++ src/modules/awake/Awake/Core/TrayHelper.cs | 8 ++++---- src/modules/awake/Awake/Program.cs | 18 ++++++++---------- 3 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 src/modules/awake/Awake/Core/Constants.cs diff --git a/src/modules/awake/Awake/Core/Constants.cs b/src/modules/awake/Awake/Core/Constants.cs new file mode 100644 index 00000000000..4e74f21cb92 --- /dev/null +++ b/src/modules/awake/Awake/Core/Constants.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace Awake.Core +{ + internal static class Constants + { + internal const string AppName = "Awake"; + internal const string FullAppName = "PowerToys " + AppName; + } +} diff --git a/src/modules/awake/Awake/Core/TrayHelper.cs b/src/modules/awake/Awake/Core/TrayHelper.cs index d8b814c819c..55f878b7fdd 100644 --- a/src/modules/awake/Awake/Core/TrayHelper.cs +++ b/src/modules/awake/Awake/Core/TrayHelper.cs @@ -57,10 +57,10 @@ internal static void SetTray(string text, AwakeSettings settings) text, settings.Properties.KeepDisplayOn, settings.Properties.Mode, - PassiveKeepAwakeCallback(text), - IndefiniteKeepAwakeCallback(text), - TimedKeepAwakeCallback(text), - KeepDisplayOnCallback(text), + PassiveKeepAwakeCallback(Constants.AppName), + IndefiniteKeepAwakeCallback(Constants.AppName), + TimedKeepAwakeCallback(Constants.AppName), + KeepDisplayOnCallback(Constants.AppName), ExitCallback()); } diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index 3b62774c7fc..a411162b573 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -27,8 +27,6 @@ namespace Awake internal class Program { private static Mutex? _mutex = null; - private const string AppName = "Awake"; - private const string FullAppName = "PowerToys " + AppName; private static FileSystemWatcher? _watcher = null; private static SettingsUtils? _settingsUtils = null; @@ -45,11 +43,11 @@ internal class Program private static int Main(string[] args) { bool instantiated; - LockMutex = new Mutex(true, AppName, out instantiated); + LockMutex = new Mutex(true, Constants.AppName, out instantiated); if (!instantiated) { - ForceExit(AppName + " is already running! Exiting the application.", 1); + ForceExit(Constants.AppName + " is already running! Exiting the application.", 1); } _log = LogManager.GetCurrentClassLogger(); @@ -122,7 +120,7 @@ private static int Main(string[] args) pidOption, }; - rootCommand.Description = AppName; + rootCommand.Description = Constants.AppName; rootCommand.Handler = CommandHandler.Create(HandleCommandLineArguments); @@ -173,10 +171,10 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, try { #pragma warning disable CS8604 // Possible null reference argument. - TrayHelper.InitializeTray(FullAppName, new Icon(Application.GetResourceStream(new Uri("/Images/Awake.ico", UriKind.Relative)).Stream)); + TrayHelper.InitializeTray(Constants.FullAppName, new Icon(Application.GetResourceStream(new Uri("/Images/Awake.ico", UriKind.Relative)).Stream)); #pragma warning restore CS8604 // Possible null reference argument. - var settingsPath = _settingsUtils.GetSettingsFilePath(AppName); + var settingsPath = _settingsUtils.GetSettingsFilePath(Constants.AppName); _log.Info($"Reading configuration file: {settingsPath}"); _watcher = new FileSystemWatcher @@ -202,7 +200,7 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, .Select(e => e.EventArgs) .Subscribe(HandleAwakeConfigChange); - TrayHelper.SetTray(FullAppName, new AwakeSettings()); + TrayHelper.SetTray(Constants.FullAppName, new AwakeSettings()); // Initially the file might not be updated, so we need to start processing // settings right away. @@ -257,7 +255,7 @@ private static void ProcessSettings() { try { - AwakeSettings settings = _settingsUtils.GetSettings(AppName); + AwakeSettings settings = _settingsUtils.GetSettings(Constants.AppName); if (settings != null) { @@ -294,7 +292,7 @@ private static void ProcessSettings() } } - TrayHelper.SetTray(FullAppName, settings); + TrayHelper.SetTray(Constants.FullAppName, settings); } else { From aa7cc5bc04f47058c84a451b81e7b8d85e7e91ed Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Sun, 4 Jul 2021 19:27:56 -0700 Subject: [PATCH 06/18] Fix process termination logic --- src/modules/awake/Awake/Core/APIHelper.cs | 2 ++ src/modules/awake/Awake/Program.cs | 11 ++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/modules/awake/Awake/Core/APIHelper.cs b/src/modules/awake/Awake/Core/APIHelper.cs index 85c02352808..78ab63141a0 100644 --- a/src/modules/awake/Awake/Core/APIHelper.cs +++ b/src/modules/awake/Awake/Core/APIHelper.cs @@ -253,6 +253,8 @@ private static bool RunTimedLoop(uint seconds, bool keepDisplayOn = true) { _log.Info($"Initiated temporary keep awake in background thread: {GetCurrentThreadId()}. Screen on: {keepDisplayOn}"); var startTime = DateTime.UtcNow; + + // TODO: Replace this with a timer. while (DateTime.UtcNow - startTime < TimeSpan.FromSeconds(Math.Abs(seconds))) { if (_threadToken.IsCancellationRequested) diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index a411162b573..d8efc32d859 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -42,6 +42,10 @@ internal class Program private static int Main(string[] args) { + // Log initialization needs to always happen before we test whether + // only one instance of Awake is running. + _log = LogManager.GetCurrentClassLogger(); + bool instantiated; LockMutex = new Mutex(true, Constants.AppName, out instantiated); @@ -50,7 +54,6 @@ private static int Main(string[] args) ForceExit(Constants.AppName + " is already running! Exiting the application.", 1); } - _log = LogManager.GetCurrentClassLogger(); _settingsUtils = new SettingsUtils(); _log.Info("Launching PowerToys Awake..."); @@ -145,7 +148,10 @@ private static void ForceExit(string message, int exitCode) APIHelper.SetNoKeepAwake(); TrayHelper.ClearTray(); - Environment.Exit(exitCode); + // Because we are running a message loop for the tray, we can't just use Environment.Exit, + // but have to make sure that we properly send the termination message. + var cwResult = System.Diagnostics.Process.GetCurrentProcess().CloseMainWindow(); + _log.Info($"Request to close main window statius: {cwResult}"); } private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, uint timeLimit, int pid) @@ -240,7 +246,6 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, private static void SetupIndefiniteKeepAwake(bool displayOn) { - // Indefinite keep awake. APIHelper.SetIndefiniteKeepAwake(LogCompletedKeepAwakeThread, LogUnexpectedOrCancelledKeepAwakeThreadCompletion, displayOn); } From 78fcaf0c269131e48447ff00b694277d6edb8de1 Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Sun, 4 Jul 2021 22:05:04 -0700 Subject: [PATCH 07/18] Making some tweaks to the termination logic --- src/modules/awake/Awake/Program.cs | 19 ++++++++++------ .../awake/AwakeModuleInterface/dllmain.cpp | 22 +++++++++---------- .../ViewModels/AwakeViewModel.cs | 4 ++-- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index d8efc32d859..2c37f866247 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -51,7 +51,7 @@ private static int Main(string[] args) if (!instantiated) { - ForceExit(Constants.AppName + " is already running! Exiting the application.", 1); + Exit(Constants.AppName + " is already running! Exiting the application.", 1, true); } _settingsUtils = new SettingsUtils(); @@ -136,12 +136,12 @@ private static bool ExitHandler(ControlType ctrlType) { _log.Info($"Exited through handler with control type: {ctrlType}"); - ForceExit("Exiting from the internal termination handler.", Environment.ExitCode); + Exit("Exiting from the internal termination handler.", Environment.ExitCode); return false; } - private static void ForceExit(string message, int exitCode) + private static void Exit(string message, int exitCode, bool force = false) { _log.Info(message); @@ -152,17 +152,22 @@ private static void ForceExit(string message, int exitCode) // but have to make sure that we properly send the termination message. var cwResult = System.Diagnostics.Process.GetCurrentProcess().CloseMainWindow(); _log.Info($"Request to close main window statius: {cwResult}"); + + if (force) + { + Environment.Exit(exitCode); + } } private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, uint timeLimit, int pid) { + _handler += new ConsoleEventHandler(ExitHandler); + APIHelper.SetConsoleControlHandler(_handler, true); + if (pid == 0) { _log.Info("No PID specified. Allocating console..."); APIHelper.AllocateConsole(); - - _handler += new ConsoleEventHandler(ExitHandler); - APIHelper.SetConsoleControlHandler(_handler, true); } _log.Info($"The value for --use-pt-config is: {usePtConfig}"); @@ -237,7 +242,7 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, { RunnerHelper.WaitForPowerToysRunner(pid, () => { - ForceExit("Terminating from PowerToys binding hook.", 0); + Exit("Terminating from PowerToys binding hook.", 0, true); }); } diff --git a/src/modules/awake/AwakeModuleInterface/dllmain.cpp b/src/modules/awake/AwakeModuleInterface/dllmain.cpp index cd9e89cad7e..223e4e364f0 100644 --- a/src/modules/awake/AwakeModuleInterface/dllmain.cpp +++ b/src/modules/awake/AwakeModuleInterface/dllmain.cpp @@ -51,13 +51,13 @@ class Awake : public PowertoyModuleIface // The PowerToy state. bool m_enabled = false; - HANDLE m_hProcess; - HANDLE send_telemetry_event; // Handle to event used to invoke PowerToys Awake HANDLE m_hInvokeEvent; + PROCESS_INFORMATION p_info; + bool is_process_running() { return WaitForSingleObject(m_hProcess, 0) == WAIT_TIMEOUT; @@ -69,22 +69,19 @@ class Awake : public PowertoyModuleIface unsigned long powertoys_pid = GetCurrentProcessId(); std::wstring executable_args = L"--use-pt-config --pid " + std::to_wstring(powertoys_pid); + std::wstring application_path = L"modules\\Awake\\PowerToys.Awake.exe"; + std::wstring full_command_path = application_path + L" " + executable_args.data(); Logger::trace(L"PowerToys Awake launching with parameters: " + executable_args); - SHELLEXECUTEINFOW sei{ sizeof(sei) }; - sei.fMask = { SEE_MASK_NOCLOSEPROCESS | SEE_MASK_FLAG_NO_UI }; - sei.lpFile = L"modules\\Awake\\PowerToys.Awake.exe"; - sei.nShow = SW_SHOWNORMAL; - sei.lpParameters = executable_args.data(); - if (!ShellExecuteExW(&sei)) + STARTUPINFO info = { sizeof(info) }; + + if (!CreateProcess(application_path.c_str(), full_command_path.data(), NULL, NULL, true, NULL, NULL, NULL, &info, &p_info)) { DWORD error = GetLastError(); - std::wstring message = L"PowerToys Awake failed to start with error = "; + std::wstring message = L"PowerToys Awake failed to start with error: "; message += std::to_wstring(error); Logger::error(message); } - - m_hProcess = sei.hProcess; } public: @@ -162,7 +159,8 @@ class Awake : public PowertoyModuleIface { ResetEvent(send_telemetry_event); ResetEvent(m_hInvokeEvent); - TerminateProcess(m_hProcess, 1); + TerminateProcess(p_info.hProcess, 1); + CloseHandle(p_info.hProcess); } m_enabled = false; diff --git a/src/settings-ui/Microsoft.PowerToys.Settings.UI.Library/ViewModels/AwakeViewModel.cs b/src/settings-ui/Microsoft.PowerToys.Settings.UI.Library/ViewModels/AwakeViewModel.cs index 357631b5ac1..e283f1190e4 100644 --- a/src/settings-ui/Microsoft.PowerToys.Settings.UI.Library/ViewModels/AwakeViewModel.cs +++ b/src/settings-ui/Microsoft.PowerToys.Settings.UI.Library/ViewModels/AwakeViewModel.cs @@ -58,7 +58,7 @@ public bool IsEnabled OnPropertyChanged(nameof(IsEnabled)); OnPropertyChanged(nameof(IsTimeConfigurationEnabled)); - var outgoing = new OutGoingGeneralSettings(GeneralSettingsConfig); + OutGoingGeneralSettings outgoing = new OutGoingGeneralSettings(GeneralSettingsConfig); SendConfigMSG(outgoing.ToString()); NotifyPropertyChanged(); } @@ -143,7 +143,7 @@ public void NotifyPropertyChanged([CallerMemberName] string propertyName = null) SndAwakeSettings outsettings = new SndAwakeSettings(Settings); SndModuleSettings ipcMessage = new SndModuleSettings(outsettings); - var targetMessage = ipcMessage.ToJsonString(); + string targetMessage = ipcMessage.ToJsonString(); SendConfigMSG(targetMessage); } } From 1bb020a4e6448f755200a8e709d2fc5647b0430f Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Sun, 4 Jul 2021 22:36:36 -0700 Subject: [PATCH 08/18] Update how the process is tracked --- src/modules/awake/AwakeModuleInterface/dllmain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/awake/AwakeModuleInterface/dllmain.cpp b/src/modules/awake/AwakeModuleInterface/dllmain.cpp index 223e4e364f0..7d0a2d2e86e 100644 --- a/src/modules/awake/AwakeModuleInterface/dllmain.cpp +++ b/src/modules/awake/AwakeModuleInterface/dllmain.cpp @@ -60,7 +60,7 @@ class Awake : public PowertoyModuleIface bool is_process_running() { - return WaitForSingleObject(m_hProcess, 0) == WAIT_TIMEOUT; + return WaitForSingleObject(p_info.hProcess, 0) == WAIT_TIMEOUT; } void launch_process() From f18bd8f07c930cccf59ec63cc77d12cc5d9d8cff Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 6 Jul 2021 13:49:43 -0700 Subject: [PATCH 09/18] Updating how application closing is done for Awake. --- src/modules/awake/Awake/Core/TrayHelper.cs | 6 +++ src/modules/awake/Awake/Program.cs | 4 +- .../awake/AwakeModuleInterface/dllmain.cpp | 43 +++++++++++++++++-- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/modules/awake/Awake/Core/TrayHelper.cs b/src/modules/awake/Awake/Core/TrayHelper.cs index 55f878b7fdd..94a33eff6f8 100644 --- a/src/modules/awake/Awake/Core/TrayHelper.cs +++ b/src/modules/awake/Awake/Core/TrayHelper.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using System.Windows.Forms; using Microsoft.PowerToys.Settings.UI.Library; +using NLog; #pragma warning disable CS8602 // Dereference of a possibly null reference. #pragma warning disable CS8603 // Possible null reference return. @@ -17,6 +18,8 @@ namespace Awake.Core { internal static class TrayHelper { + private static readonly Logger _log; + private static NotifyIcon? trayIcon; private static NotifyIcon TrayIcon { get => trayIcon; set => trayIcon = value; } @@ -27,6 +30,7 @@ internal static class TrayHelper static TrayHelper() { + _log = LogManager.GetCurrentClassLogger(); TrayIcon = new NotifyIcon(); ModuleSettings = new SettingsUtils(); } @@ -41,7 +45,9 @@ public static void InitializeTray(string text, Icon icon, ContextMenuStrip? cont ((NotifyIcon?)tray).ContextMenuStrip = contextMenu; ((NotifyIcon?)tray).Visible = true; + _log.Info("Setting up the tray."); Application.Run(); + _log.Info("Tray setup complete."); }, TrayIcon); } diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index 2c37f866247..d7735d104ac 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -151,7 +151,7 @@ private static void Exit(string message, int exitCode, bool force = false) // Because we are running a message loop for the tray, we can't just use Environment.Exit, // but have to make sure that we properly send the termination message. var cwResult = System.Diagnostics.Process.GetCurrentProcess().CloseMainWindow(); - _log.Info($"Request to close main window statius: {cwResult}"); + _log.Info($"Request to close main window status: {cwResult}"); if (force) { @@ -313,7 +313,7 @@ private static void ProcessSettings() } catch (Exception ex) { - var errorMessage = $"There was a problem reading the configuration file. Error: {ex.GetType()} {ex.Message}"; + string? errorMessage = $"There was a problem reading the configuration file. Error: {ex.GetType()} {ex.Message}"; _log.Info(errorMessage); _log.Debug(errorMessage); } diff --git a/src/modules/awake/AwakeModuleInterface/dllmain.cpp b/src/modules/awake/AwakeModuleInterface/dllmain.cpp index 7d0a2d2e86e..bfc30a181e5 100644 --- a/src/modules/awake/AwakeModuleInterface/dllmain.cpp +++ b/src/modules/awake/AwakeModuleInterface/dllmain.cpp @@ -15,6 +15,7 @@ #include #include +#include BOOL APIENTRY DllMain(HMODULE hModule, DWORD ul_reason_for_call, LPVOID lpReserved) { @@ -157,20 +158,56 @@ class Awake : public PowertoyModuleIface { if (m_enabled) { + Logger::trace(L"Disabling Awake..."); ResetEvent(send_telemetry_event); ResetEvent(m_hInvokeEvent); - TerminateProcess(p_info.hProcess, 1); - CloseHandle(p_info.hProcess); + + // Ensures that we close the existing windows, which tackles + // the problem of the "hanging tray" - icons that are still visible + // even though the process is no longer running. + for (HWND handleId : EnumerateWindowHandles(p_info.dwProcessId)) + { + Logger::trace(L"Sending window close to handle ID: " + HWNDToString(handleId)); + SendMessage(handleId, WM_CLOSE, 0, 0); + } + + if (!TerminateProcess(p_info.hProcess, 1)) + { + Logger::trace(L"Terminated the Awake process."); + CloseHandle(p_info.hProcess); + } } m_enabled = false; } - // Returns if the powertoys is enabled virtual bool is_enabled() override { return m_enabled; } + + std::wstring HWNDToString(HWND sourceHwnd) + { + TCHAR hwndBuffer[64]; + _stprintf(hwndBuffer, _T("%p"), sourceHwnd); + return hwndBuffer; + + } + + std::set EnumerateWindowHandles(DWORD processId) + { + std::set handles; + for (HWND hwnd = GetTopWindow(NULL); hwnd; hwnd = GetNextWindow(hwnd, GW_HWNDNEXT)) + { + DWORD dwWindowProcessID; + DWORD dwThreadID = GetWindowThreadProcessId(hwnd, &dwWindowProcessID); + if (dwWindowProcessID == processId) + { + handles.emplace(hwnd); + } + } + return handles; + } }; extern "C" __declspec(dllexport) PowertoyModuleIface* __cdecl powertoy_create() From 24048753aee99760a95b7982cdb6395368135742 Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 6 Jul 2021 13:58:43 -0700 Subject: [PATCH 10/18] Update with explicit types --- src/modules/awake/Awake/Program.cs | 31 +++++++++---------- .../awake/AwakeModuleInterface/dllmain.cpp | 2 +- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index d7735d104ac..61bf0fa6e7b 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -46,8 +46,7 @@ private static int Main(string[] args) // only one instance of Awake is running. _log = LogManager.GetCurrentClassLogger(); - bool instantiated; - LockMutex = new Mutex(true, Constants.AppName, out instantiated); + LockMutex = new Mutex(true, Constants.AppName, out bool instantiated); if (!instantiated) { @@ -63,7 +62,7 @@ private static int Main(string[] args) _log.Info("Parsing parameters..."); - var configOption = new Option( + Option? configOption = new Option( aliases: new[] { "--use-pt-config", "-c" }, getDefaultValue: () => false, description: "Specifies whether PowerToys Awake will be using the PowerToys configuration file for managing the state.") @@ -76,7 +75,7 @@ private static int Main(string[] args) configOption.Required = false; - var displayOption = new Option( + Option? displayOption = new Option( aliases: new[] { "--display-on", "-d" }, getDefaultValue: () => true, description: "Determines whether the display should be kept awake.") @@ -89,7 +88,7 @@ private static int Main(string[] args) displayOption.Required = false; - var timeOption = new Option( + Option? timeOption = new Option( aliases: new[] { "--time-limit", "-t" }, getDefaultValue: () => 0, description: "Determines the interval, in seconds, during which the computer is kept awake.") @@ -102,7 +101,7 @@ private static int Main(string[] args) timeOption.Required = false; - var pidOption = new Option( + Option? pidOption = new Option( aliases: new[] { "--pid", "-p" }, getDefaultValue: () => 0, description: "Bind the execution of PowerToys Awake to another process.") @@ -115,7 +114,7 @@ private static int Main(string[] args) pidOption.Required = false; - var rootCommand = new RootCommand + RootCommand? rootCommand = new RootCommand { configOption, displayOption, @@ -150,7 +149,7 @@ private static void Exit(string message, int exitCode, bool force = false) // Because we are running a message loop for the tray, we can't just use Environment.Exit, // but have to make sure that we properly send the termination message. - var cwResult = System.Diagnostics.Process.GetCurrentProcess().CloseMainWindow(); + bool cwResult = System.Diagnostics.Process.GetCurrentProcess().CloseMainWindow(); _log.Info($"Request to close main window status: {cwResult}"); if (force) @@ -185,7 +184,7 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, TrayHelper.InitializeTray(Constants.FullAppName, new Icon(Application.GetResourceStream(new Uri("/Images/Awake.ico", UriKind.Relative)).Stream)); #pragma warning restore CS8604 // Possible null reference argument. - var settingsPath = _settingsUtils.GetSettingsFilePath(Constants.AppName); + string? settingsPath = _settingsUtils.GetSettingsFilePath(Constants.AppName); _log.Info($"Reading configuration file: {settingsPath}"); _watcher = new FileSystemWatcher @@ -196,15 +195,15 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, Filter = Path.GetFileName(settingsPath), }; - var changedObservable = Observable.FromEventPattern( + IObservable>? changedObservable = Observable.FromEventPattern( h => _watcher.Changed += h, h => _watcher.Changed -= h); - var createdObservable = Observable.FromEventPattern( + IObservable>? createdObservable = Observable.FromEventPattern( cre => _watcher.Created += cre, cre => _watcher.Created -= cre); - var mergedObservable = Observable.Merge(changedObservable, createdObservable); + IObservable>? mergedObservable = Observable.Merge(changedObservable, createdObservable); mergedObservable.Throttle(TimeSpan.FromMilliseconds(25)) .SubscribeOn(TaskPoolScheduler.Default) @@ -219,14 +218,14 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, } catch (Exception ex) { - var errorString = $"There was a problem with the configuration file. Make sure it exists.\n{ex.Message}"; + string? errorString = $"There was a problem with the configuration file. Make sure it exists.\n{ex.Message}"; _log.Info(errorString); _log.Debug(errorString); } } else { - var mode = timeLimit <= 0 ? AwakeMode.INDEFINITE : AwakeMode.TIMED; + AwakeMode mode = timeLimit <= 0 ? AwakeMode.INDEFINITE : AwakeMode.TIMED; if (mode == AwakeMode.INDEFINITE) { @@ -306,7 +305,7 @@ private static void ProcessSettings() } else { - var errorMessage = "Settings are null."; + string? errorMessage = "Settings are null."; _log.Info(errorMessage); _log.Debug(errorMessage); } @@ -335,7 +334,7 @@ private static void SetupTimedKeepAwake(uint time, bool displayOn) private static void LogUnexpectedOrCancelledKeepAwakeThreadCompletion() { - var errorMessage = "The keep-awake thread was terminated early."; + string? errorMessage = "The keep-awake thread was terminated early."; _log.Info(errorMessage); _log.Debug(errorMessage); } diff --git a/src/modules/awake/AwakeModuleInterface/dllmain.cpp b/src/modules/awake/AwakeModuleInterface/dllmain.cpp index bfc30a181e5..3e118840b9c 100644 --- a/src/modules/awake/AwakeModuleInterface/dllmain.cpp +++ b/src/modules/awake/AwakeModuleInterface/dllmain.cpp @@ -189,7 +189,7 @@ class Awake : public PowertoyModuleIface std::wstring HWNDToString(HWND sourceHwnd) { TCHAR hwndBuffer[64]; - _stprintf(hwndBuffer, _T("%p"), sourceHwnd); + swprintf_s(hwndBuffer, _T("%p"), sourceHwnd); return hwndBuffer; } From d8e29731343d353da0daa95d99736ec926cb0e76 Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 6 Jul 2021 14:47:15 -0700 Subject: [PATCH 11/18] Fix high CPU usage for timed keep awake. --- src/modules/awake/Awake/Core/APIHelper.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/modules/awake/Awake/Core/APIHelper.cs b/src/modules/awake/Awake/Core/APIHelper.cs index 78ab63141a0..7092f49f1f3 100644 --- a/src/modules/awake/Awake/Core/APIHelper.cs +++ b/src/modules/awake/Awake/Core/APIHelper.cs @@ -252,16 +252,17 @@ private static bool RunTimedLoop(uint seconds, bool keepDisplayOn = true) if (success) { _log.Info($"Initiated temporary keep awake in background thread: {GetCurrentThreadId()}. Screen on: {keepDisplayOn}"); - var startTime = DateTime.UtcNow; - // TODO: Replace this with a timer. - while (DateTime.UtcNow - startTime < TimeSpan.FromSeconds(Math.Abs(seconds))) + System.Timers.Timer timedLoopTimer = new System.Timers.Timer(seconds * 1000); + timedLoopTimer.Elapsed += (s, e) => { - if (_threadToken.IsCancellationRequested) - { - _threadToken.ThrowIfCancellationRequested(); - } - } + _tokenSource.Cancel(); + + timedLoopTimer.Stop(); + }; + timedLoopTimer.Start(); + + WaitHandle.WaitAny(new[] { _threadToken.WaitHandle }); return success; } From 6a89042bd0f05adeb254d168ab73e12e8b91b78e Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 6 Jul 2021 15:03:22 -0700 Subject: [PATCH 12/18] Logging typo fix. --- src/modules/awake/Awake/Core/APIHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/awake/Awake/Core/APIHelper.cs b/src/modules/awake/Awake/Core/APIHelper.cs index 7092f49f1f3..f112f961351 100644 --- a/src/modules/awake/Awake/Core/APIHelper.cs +++ b/src/modules/awake/Awake/Core/APIHelper.cs @@ -185,7 +185,7 @@ public static void SetTimedKeepAwake(uint seconds, Action callback, Action } catch (OperationCanceledException) { - _log.Info("Confirmed background thread cancellation when setting indefinite keep awake."); + _log.Info("Confirmed background thread cancellation when setting timed keep awake."); } _tokenSource = new CancellationTokenSource(); From f0e5b63f38a7bcafad51892e8d42b93930b4d231 Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 6 Jul 2021 15:43:38 -0700 Subject: [PATCH 13/18] Update some of the timer logic. --- src/modules/awake/Awake/Core/APIHelper.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/modules/awake/Awake/Core/APIHelper.cs b/src/modules/awake/Awake/Core/APIHelper.cs index f112f961351..844791562b4 100644 --- a/src/modules/awake/Awake/Core/APIHelper.cs +++ b/src/modules/awake/Awake/Core/APIHelper.cs @@ -49,6 +49,7 @@ public class APIHelper private static CancellationToken _threadToken; private static Task? _runnerThread; + private static System.Timers.Timer _timedLoopTimer; [DllImport("kernel32.dll", SetLastError = true)] private static extern bool SetConsoleCtrlHandler(ConsoleEventHandler handler, bool add); @@ -82,6 +83,7 @@ private static extern IntPtr CreateFile( static APIHelper() { + _timedLoopTimer = new System.Timers.Timer(); _log = LogManager.GetCurrentClassLogger(); _tokenSource = new CancellationTokenSource(); } @@ -253,16 +255,24 @@ private static bool RunTimedLoop(uint seconds, bool keepDisplayOn = true) { _log.Info($"Initiated temporary keep awake in background thread: {GetCurrentThreadId()}. Screen on: {keepDisplayOn}"); - System.Timers.Timer timedLoopTimer = new System.Timers.Timer(seconds * 1000); - timedLoopTimer.Elapsed += (s, e) => + _timedLoopTimer = new System.Timers.Timer(seconds * 1000); + _timedLoopTimer.Elapsed += (s, e) => { _tokenSource.Cancel(); - timedLoopTimer.Stop(); + _timedLoopTimer.Stop(); }; - timedLoopTimer.Start(); + + _timedLoopTimer.Disposed += (s, e) => + { + _log.Info("Old timer disposed."); + }; + + _timedLoopTimer.Start(); WaitHandle.WaitAny(new[] { _threadToken.WaitHandle }); + _timedLoopTimer.Stop(); + _timedLoopTimer.Dispose(); return success; } From d7e5d2e564bd72fbec34eaf6a6ce90287f1cacff Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 6 Jul 2021 15:45:30 -0700 Subject: [PATCH 14/18] Fix variable naming for consistency --- src/modules/awake/Awake/Core/TrayHelper.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modules/awake/Awake/Core/TrayHelper.cs b/src/modules/awake/Awake/Core/TrayHelper.cs index 94a33eff6f8..85d68066af1 100644 --- a/src/modules/awake/Awake/Core/TrayHelper.cs +++ b/src/modules/awake/Awake/Core/TrayHelper.cs @@ -20,13 +20,13 @@ internal static class TrayHelper { private static readonly Logger _log; - private static NotifyIcon? trayIcon; + private static NotifyIcon? _trayIcon; - private static NotifyIcon TrayIcon { get => trayIcon; set => trayIcon = value; } + private static NotifyIcon TrayIcon { get => _trayIcon; set => _trayIcon = value; } - private static SettingsUtils? moduleSettings; + private static SettingsUtils? _moduleSettings; - private static SettingsUtils ModuleSettings { get => moduleSettings; set => moduleSettings = value; } + private static SettingsUtils ModuleSettings { get => _moduleSettings; set => _moduleSettings = value; } static TrayHelper() { From 1644a13470939d134ae7074cf001118ffcdad3f9 Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Tue, 6 Jul 2021 16:19:38 -0700 Subject: [PATCH 15/18] Cleanup the C++ interface --- .../awake/AwakeModuleInterface/dllmain.cpp | 67 +++++++------------ 1 file changed, 25 insertions(+), 42 deletions(-) diff --git a/src/modules/awake/AwakeModuleInterface/dllmain.cpp b/src/modules/awake/AwakeModuleInterface/dllmain.cpp index 3e118840b9c..0a0fb1a00a1 100644 --- a/src/modules/awake/AwakeModuleInterface/dllmain.cpp +++ b/src/modules/awake/AwakeModuleInterface/dllmain.cpp @@ -34,29 +34,18 @@ BOOL APIENTRY DllMain(HMODULE hModule, DWORD ul_reason_for_call, LPVOID lpReserv return TRUE; } -// The PowerToy name that will be shown in the settings. const static wchar_t* MODULE_NAME = L"Awake"; - -// Add a description that will we shown in the module settings page. const static wchar_t* MODULE_DESC = L"A module that keeps your computer awake on-demand."; -// Implement the PowerToy Module Interface and all the required methods. class Awake : public PowertoyModuleIface { std::wstring app_name; - - // Contains the non localized key of the powertoy std::wstring app_key; private: - // The PowerToy state. bool m_enabled = false; - HANDLE send_telemetry_event; - - // Handle to event used to invoke PowerToys Awake HANDLE m_hInvokeEvent; - PROCESS_INFORMATION p_info; bool is_process_running() @@ -85,8 +74,31 @@ class Awake : public PowertoyModuleIface } } + std::wstring HWNDToString(HWND sourceHwnd) + { + TCHAR hwndBuffer[64]; + swprintf_s(hwndBuffer, _T("%p"), sourceHwnd); + return hwndBuffer; + } + + // Returns the list of window handles for a given process. Used to clean up + // the tray in the PowerToys Awake process that was spawned from the PowerToys runner. + std::set EnumerateWindowHandles(DWORD processId) + { + std::set handles; + for (HWND hwnd = GetTopWindow(NULL); hwnd; hwnd = GetNextWindow(hwnd, GW_HWNDNEXT)) + { + DWORD dwWindowProcessID; + DWORD dwThreadID = GetWindowThreadProcessId(hwnd, &dwWindowProcessID); + if (dwWindowProcessID == processId) + { + handles.emplace(hwnd); + } + } + return handles; + } + public: - // Constructor Awake() { app_name = GET_RESOURCE_STRING(IDS_AWAKE_NAME); @@ -97,37 +109,31 @@ class Awake : public PowertoyModuleIface Logger::info("Launcher object is constructing"); }; - // Destroy the powertoy and free memory virtual void destroy() override { delete this; } - // Return the display name of the powertoy, this will be cached by the runner virtual const wchar_t* get_name() override { return MODULE_NAME; } - // Return JSON with the configuration options. virtual bool get_config(wchar_t* buffer, int* buffer_size) override { HINSTANCE hinstance = reinterpret_cast(&__ImageBase); - // Create a Settings object. PowerToysSettings::Settings settings(hinstance, get_name()); settings.set_description(MODULE_DESC); return settings.serialize_to_buffer(buffer, buffer_size); } - // Return the non localized key of the powertoy, this will be cached by the runner virtual const wchar_t* get_key() override { return app_key.c_str(); } - // Called by the runner to pass the updated settings values as a serialized JSON. virtual void set_config(const wchar_t* config) override { try @@ -137,7 +143,7 @@ class Awake : public PowertoyModuleIface PowerToysSettings::PowerToyValues::from_json_string(config, get_key()); // If you don't need to do any custom processing of the settings, proceed - // to persists the values calling: + // to persists the values. values.save_to_settings_file(); } catch (std::exception&) @@ -185,29 +191,6 @@ class Awake : public PowertoyModuleIface { return m_enabled; } - - std::wstring HWNDToString(HWND sourceHwnd) - { - TCHAR hwndBuffer[64]; - swprintf_s(hwndBuffer, _T("%p"), sourceHwnd); - return hwndBuffer; - - } - - std::set EnumerateWindowHandles(DWORD processId) - { - std::set handles; - for (HWND hwnd = GetTopWindow(NULL); hwnd; hwnd = GetNextWindow(hwnd, GW_HWNDNEXT)) - { - DWORD dwWindowProcessID; - DWORD dwThreadID = GetWindowThreadProcessId(hwnd, &dwWindowProcessID); - if (dwWindowProcessID == processId) - { - handles.emplace(hwnd); - } - } - return handles; - } }; extern "C" __declspec(dllexport) PowertoyModuleIface* __cdecl powertoy_create() From 77c9b919ec44fd4a95fe03ea37064ad285a4e4fc Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Thu, 8 Jul 2021 19:25:10 -0700 Subject: [PATCH 16/18] Cleanup the code a bit This change introduces support for: - Proper event handling across the two apps (Awake and runner). - Removal of unnecessary functions. --- src/common/interop/interop.cpp | 4 +++ src/common/interop/shared_constants.h | 3 ++ src/modules/awake/Awake/Core/APIHelper.cs | 9 ------ .../{Constants.cs => InternalConstants.cs} | 2 +- src/modules/awake/Awake/Core/TrayHelper.cs | 8 +++--- src/modules/awake/Awake/Program.cs | 28 ++++++++++++------- .../awake/AwakeModuleInterface/dllmain.cpp | 25 +++++++++++------ 7 files changed, 46 insertions(+), 33 deletions(-) rename src/modules/awake/Awake/Core/{Constants.cs => InternalConstants.cs} (88%) diff --git a/src/common/interop/interop.cpp b/src/common/interop/interop.cpp index 09529e9d1c5..6dd4dc151be 100644 --- a/src/common/interop/interop.cpp +++ b/src/common/interop/interop.cpp @@ -186,5 +186,9 @@ public static String ^ ShowColorPickerSharedEvent() { return gcnew String(CommonSharedConstants::SHOW_COLOR_PICKER_SHARED_EVENT); } + + static String ^ AwakeExitEvent() { + return gcnew String(CommonSharedConstants::AWAKE_EXIT_EVENT); + } }; } diff --git a/src/common/interop/shared_constants.h b/src/common/interop/shared_constants.h index 176530ce1d4..9c882e86752 100644 --- a/src/common/interop/shared_constants.h +++ b/src/common/interop/shared_constants.h @@ -28,6 +28,9 @@ namespace CommonSharedConstants const wchar_t FANCY_ZONES_EDITOR_TOGGLE_EVENT[] = L"Local\\FancyZones-ToggleEditorEvent-1e174338-06a3-472b-874d-073b21c62f14"; + // Path to the event used by Awake + const wchar_t AWAKE_EXIT_EVENT[] = L"Local\\PowerToysAwakeExitEvent-c0d5e305-35fc-4fb5-83ec-f6070cfaf7fe"; + // Max DWORD for key code to disable keys. const DWORD VK_DISABLED = 0x100; } diff --git a/src/modules/awake/Awake/Core/APIHelper.cs b/src/modules/awake/Awake/Core/APIHelper.cs index 844791562b4..5e173807516 100644 --- a/src/modules/awake/Awake/Core/APIHelper.cs +++ b/src/modules/awake/Awake/Core/APIHelper.cs @@ -61,10 +61,6 @@ public class APIHelper [return: MarshalAs(UnmanagedType.Bool)] private static extern bool AllocConsole(); - [DllImport("kernel32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static extern bool FreeConsole(); - [DllImport("kernel32.dll", SetLastError = true)] private static extern bool SetStdHandle(int nStdHandle, IntPtr hHandle); @@ -93,11 +89,6 @@ public static void SetConsoleControlHandler(ConsoleEventHandler handler, bool ad SetConsoleCtrlHandler(handler, addHandler); } - public static void DeallocateConsole() - { - FreeConsole(); - } - public static void AllocateConsole() { _log.Debug("Bootstrapping the console allocation routine."); diff --git a/src/modules/awake/Awake/Core/Constants.cs b/src/modules/awake/Awake/Core/InternalConstants.cs similarity index 88% rename from src/modules/awake/Awake/Core/Constants.cs rename to src/modules/awake/Awake/Core/InternalConstants.cs index 4e74f21cb92..2a74eea9deb 100644 --- a/src/modules/awake/Awake/Core/Constants.cs +++ b/src/modules/awake/Awake/Core/InternalConstants.cs @@ -4,7 +4,7 @@ namespace Awake.Core { - internal static class Constants + internal static class InternalConstants { internal const string AppName = "Awake"; internal const string FullAppName = "PowerToys " + AppName; diff --git a/src/modules/awake/Awake/Core/TrayHelper.cs b/src/modules/awake/Awake/Core/TrayHelper.cs index 85d68066af1..e88dbdfcb61 100644 --- a/src/modules/awake/Awake/Core/TrayHelper.cs +++ b/src/modules/awake/Awake/Core/TrayHelper.cs @@ -63,10 +63,10 @@ internal static void SetTray(string text, AwakeSettings settings) text, settings.Properties.KeepDisplayOn, settings.Properties.Mode, - PassiveKeepAwakeCallback(Constants.AppName), - IndefiniteKeepAwakeCallback(Constants.AppName), - TimedKeepAwakeCallback(Constants.AppName), - KeepDisplayOnCallback(Constants.AppName), + PassiveKeepAwakeCallback(InternalConstants.AppName), + IndefiniteKeepAwakeCallback(InternalConstants.AppName), + TimedKeepAwakeCallback(InternalConstants.AppName), + KeepDisplayOnCallback(InternalConstants.AppName), ExitCallback()); } diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index 61bf0fa6e7b..ee7370da7a2 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -15,6 +15,7 @@ using System.Threading; using System.Windows; using Awake.Core; +using interop; using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Library; using NLog; @@ -46,11 +47,11 @@ private static int Main(string[] args) // only one instance of Awake is running. _log = LogManager.GetCurrentClassLogger(); - LockMutex = new Mutex(true, Constants.AppName, out bool instantiated); + LockMutex = new Mutex(true, InternalConstants.AppName, out bool instantiated); if (!instantiated) { - Exit(Constants.AppName + " is already running! Exiting the application.", 1, true); + Exit(InternalConstants.AppName + " is already running! Exiting the application.", 1, true); } _settingsUtils = new SettingsUtils(); @@ -122,7 +123,7 @@ private static int Main(string[] args) pidOption, }; - rootCommand.Description = Constants.AppName; + rootCommand.Description = InternalConstants.AppName; rootCommand.Handler = CommandHandler.Create(HandleCommandLineArguments); @@ -180,11 +181,18 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, // and instead watch for changes in the file. try { -#pragma warning disable CS8604 // Possible null reference argument. - TrayHelper.InitializeTray(Constants.FullAppName, new Icon(Application.GetResourceStream(new Uri("/Images/Awake.ico", UriKind.Relative)).Stream)); -#pragma warning restore CS8604 // Possible null reference argument. + new Thread(() => + { + var eventHandle = new EventWaitHandle(false, EventResetMode.AutoReset, Constants.AwakeExitEvent()); + if (eventHandle.WaitOne()) + { + Exit("Received a signal to end the process.Making sure we quit...", 0, true); + } + }).Start(); + + TrayHelper.InitializeTray(InternalConstants.FullAppName, new Icon(Application.GetResourceStream(new Uri("/Images/Awake.ico", UriKind.Relative)).Stream)); - string? settingsPath = _settingsUtils.GetSettingsFilePath(Constants.AppName); + string? settingsPath = _settingsUtils.GetSettingsFilePath(InternalConstants.AppName); _log.Info($"Reading configuration file: {settingsPath}"); _watcher = new FileSystemWatcher @@ -210,7 +218,7 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, .Select(e => e.EventArgs) .Subscribe(HandleAwakeConfigChange); - TrayHelper.SetTray(Constants.FullAppName, new AwakeSettings()); + TrayHelper.SetTray(InternalConstants.FullAppName, new AwakeSettings()); // Initially the file might not be updated, so we need to start processing // settings right away. @@ -264,7 +272,7 @@ private static void ProcessSettings() { try { - AwakeSettings settings = _settingsUtils.GetSettings(Constants.AppName); + AwakeSettings settings = _settingsUtils.GetSettings(InternalConstants.AppName); if (settings != null) { @@ -301,7 +309,7 @@ private static void ProcessSettings() } } - TrayHelper.SetTray(Constants.FullAppName, settings); + TrayHelper.SetTray(InternalConstants.FullAppName, settings); } else { diff --git a/src/modules/awake/AwakeModuleInterface/dllmain.cpp b/src/modules/awake/AwakeModuleInterface/dllmain.cpp index 0a0fb1a00a1..727986385c7 100644 --- a/src/modules/awake/AwakeModuleInterface/dllmain.cpp +++ b/src/modules/awake/AwakeModuleInterface/dllmain.cpp @@ -168,18 +168,25 @@ class Awake : public PowertoyModuleIface ResetEvent(send_telemetry_event); ResetEvent(m_hInvokeEvent); - // Ensures that we close the existing windows, which tackles - // the problem of the "hanging tray" - icons that are still visible - // even though the process is no longer running. - for (HWND handleId : EnumerateWindowHandles(p_info.dwProcessId)) + auto exitEvent = CreateEvent(nullptr, false, false, CommonSharedConstants::AWAKE_EXIT_EVENT); + if (!exitEvent) { - Logger::trace(L"Sending window close to handle ID: " + HWNDToString(handleId)); - SendMessage(handleId, WM_CLOSE, 0, 0); + Logger::warn(L"Failed to create exit event for PowerToys Awake. {}", get_last_error_or_default(GetLastError())); } - - if (!TerminateProcess(p_info.hProcess, 1)) + else { - Logger::trace(L"Terminated the Awake process."); + Logger::trace(L"Signaled exit event for PowerToys Awake."); + if (!SetEvent(exitEvent)) + { + Logger::warn(L"Failed to signal exit event for PowerToys Awake. {}", get_last_error_or_default(GetLastError())); + + // For some reason, we couldn't process the signal correctly, so we still + // need to terminate the Awake process. + TerminateProcess(p_info.hProcess, 1); + } + + ResetEvent(exitEvent); + CloseHandle(exitEvent); CloseHandle(p_info.hProcess); } } From c7a0dccf584a7f79cb96bf0a40faa1d9693523f4 Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Thu, 8 Jul 2021 19:32:59 -0700 Subject: [PATCH 17/18] Cleaning up the code even further --- src/modules/awake/Awake/Core/TrayHelper.cs | 62 +++++++--------------- src/modules/awake/Awake/Program.cs | 8 ++- 2 files changed, 23 insertions(+), 47 deletions(-) diff --git a/src/modules/awake/Awake/Core/TrayHelper.cs b/src/modules/awake/Awake/Core/TrayHelper.cs index e88dbdfcb61..96ea80bad73 100644 --- a/src/modules/awake/Awake/Core/TrayHelper.cs +++ b/src/modules/awake/Awake/Core/TrayHelper.cs @@ -166,28 +166,21 @@ private static Action IndefiniteKeepAwakeCallback(string moduleName) public static void SetTray(string text, bool keepDisplayOn, AwakeMode mode, Action passiveKeepAwakeCallback, Action indefiniteKeepAwakeCallback, Action timedKeepAwakeCallback, Action keepDisplayOnCallback, Action exitCallback) { - var contextMenuStrip = new ContextMenuStrip(); + ContextMenuStrip? contextMenuStrip = new ContextMenuStrip(); // Main toolstrip. - var operationContextMenu = new ToolStripMenuItem + ToolStripMenuItem? operationContextMenu = new ToolStripMenuItem { Text = "Mode", }; // No keep-awake menu item. - var passiveMenuItem = new ToolStripMenuItem + ToolStripMenuItem? passiveMenuItem = new ToolStripMenuItem { Text = "Off (Passive)", }; - if (mode == AwakeMode.PASSIVE) - { - passiveMenuItem.Checked = true; - } - else - { - passiveMenuItem.Checked = false; - } + passiveMenuItem.Checked = mode == AwakeMode.PASSIVE; passiveMenuItem.Click += (e, s) => { @@ -196,19 +189,12 @@ public static void SetTray(string text, bool keepDisplayOn, AwakeMode mode, Acti }; // Indefinite keep-awake menu item. - var indefiniteMenuItem = new ToolStripMenuItem + ToolStripMenuItem? indefiniteMenuItem = new ToolStripMenuItem { Text = "Keep awake indefinitely", }; - if (mode == AwakeMode.INDEFINITE) - { - indefiniteMenuItem.Checked = true; - } - else - { - indefiniteMenuItem.Checked = false; - } + indefiniteMenuItem.Checked = mode == AwakeMode.INDEFINITE; indefiniteMenuItem.Click += (e, s) => { @@ -216,18 +202,12 @@ public static void SetTray(string text, bool keepDisplayOn, AwakeMode mode, Acti indefiniteKeepAwakeCallback(); }; - var displayOnMenuItem = new ToolStripMenuItem + ToolStripMenuItem? displayOnMenuItem = new ToolStripMenuItem { Text = "Keep screen on", }; - if (keepDisplayOn) - { - displayOnMenuItem.Checked = true; - } - else - { - displayOnMenuItem.Checked = false; - } + + displayOnMenuItem.Checked = keepDisplayOn; displayOnMenuItem.Click += (e, s) => { @@ -236,43 +216,40 @@ public static void SetTray(string text, bool keepDisplayOn, AwakeMode mode, Acti }; // Timed keep-awake menu item - var timedMenuItem = new ToolStripMenuItem + ToolStripMenuItem? timedMenuItem = new ToolStripMenuItem { Text = "Keep awake temporarily", }; - if (mode == AwakeMode.TIMED) - { - timedMenuItem.Checked = true; - } - else - { - timedMenuItem.Checked = false; - } - var halfHourMenuItem = new ToolStripMenuItem + timedMenuItem.Checked = mode == AwakeMode.TIMED; + + ToolStripMenuItem? halfHourMenuItem = new ToolStripMenuItem { Text = "30 minutes", }; + halfHourMenuItem.Click += (e, s) => { // User is setting the keep-awake to 30 minutes. timedKeepAwakeCallback(0, 30); }; - var oneHourMenuItem = new ToolStripMenuItem + ToolStripMenuItem? oneHourMenuItem = new ToolStripMenuItem { Text = "1 hour", }; + oneHourMenuItem.Click += (e, s) => { // User is setting the keep-awake to 1 hour. timedKeepAwakeCallback(1, 0); }; - var twoHoursMenuItem = new ToolStripMenuItem + ToolStripMenuItem? twoHoursMenuItem = new ToolStripMenuItem { Text = "2 hours", }; + twoHoursMenuItem.Click += (e, s) => { // User is setting the keep-awake to 2 hours. @@ -280,10 +257,11 @@ public static void SetTray(string text, bool keepDisplayOn, AwakeMode mode, Acti }; // Exit menu item. - var exitContextMenu = new ToolStripMenuItem + ToolStripMenuItem? exitContextMenu = new ToolStripMenuItem { Text = "Exit", }; + exitContextMenu.Click += (e, s) => { // User is setting the keep-awake to 2 hours. diff --git a/src/modules/awake/Awake/Program.cs b/src/modules/awake/Awake/Program.cs index ee7370da7a2..d69794b4914 100644 --- a/src/modules/awake/Awake/Program.cs +++ b/src/modules/awake/Awake/Program.cs @@ -183,10 +183,10 @@ private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, { new Thread(() => { - var eventHandle = new EventWaitHandle(false, EventResetMode.AutoReset, Constants.AwakeExitEvent()); + EventWaitHandle? eventHandle = new EventWaitHandle(false, EventResetMode.AutoReset, Constants.AwakeExitEvent()); if (eventHandle.WaitOne()) { - Exit("Received a signal to end the process.Making sure we quit...", 0, true); + Exit("Received a signal to end the process. Making sure we quit...", 0, true); } }).Start(); @@ -286,14 +286,12 @@ private static void ProcessSettings() case AwakeMode.INDEFINITE: { - // Indefinite keep awake. SetupIndefiniteKeepAwake(settings.Properties.KeepDisplayOn); break; } case AwakeMode.TIMED: { - // Timed keep-awake. uint computedTime = (settings.Properties.Hours * 60 * 60) + (settings.Properties.Minutes * 60); SetupTimedKeepAwake(computedTime, settings.Properties.KeepDisplayOn); @@ -302,7 +300,7 @@ private static void ProcessSettings() default: { - var errorMessage = "Unknown mode of operation. Check config file."; + string? errorMessage = "Unknown mode of operation. Check config file."; _log.Info(errorMessage); _log.Debug(errorMessage); break; From c4d69cd673fe317bac63d15447713ab86656f01c Mon Sep 17 00:00:00 2001 From: Den Delimarsky <1389609+dend@users.noreply.github.com> Date: Fri, 9 Jul 2021 08:59:28 -0700 Subject: [PATCH 18/18] Remove unnecessary functions --- .../awake/AwakeModuleInterface/dllmain.cpp | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/src/modules/awake/AwakeModuleInterface/dllmain.cpp b/src/modules/awake/AwakeModuleInterface/dllmain.cpp index 727986385c7..4b5859e3b2f 100644 --- a/src/modules/awake/AwakeModuleInterface/dllmain.cpp +++ b/src/modules/awake/AwakeModuleInterface/dllmain.cpp @@ -74,30 +74,6 @@ class Awake : public PowertoyModuleIface } } - std::wstring HWNDToString(HWND sourceHwnd) - { - TCHAR hwndBuffer[64]; - swprintf_s(hwndBuffer, _T("%p"), sourceHwnd); - return hwndBuffer; - } - - // Returns the list of window handles for a given process. Used to clean up - // the tray in the PowerToys Awake process that was spawned from the PowerToys runner. - std::set EnumerateWindowHandles(DWORD processId) - { - std::set handles; - for (HWND hwnd = GetTopWindow(NULL); hwnd; hwnd = GetNextWindow(hwnd, GW_HWNDNEXT)) - { - DWORD dwWindowProcessID; - DWORD dwThreadID = GetWindowThreadProcessId(hwnd, &dwWindowProcessID); - if (dwWindowProcessID == processId) - { - handles.emplace(hwnd); - } - } - return handles; - } - public: Awake() {