From 227c5d8147f60564cf5f080719fb841409a7933f Mon Sep 17 00:00:00 2001 From: Davide Giacometti <25966642+davidegiacometti@users.noreply.github.com> Date: Mon, 14 Jul 2025 02:50:45 +0200 Subject: [PATCH] [Peek] Terminate Preview Handlers Processes (#40116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request This is an attempt to release Preview Handlers instantiated by Peek and close the related processe. ⚠️ Note that even if the PR improve the current behavior, the solution doesn't work 100% of times. I noticed that sometimes the process gets leaked also when Preview Handler is used in Explorer 🤔 ## PR Checklist - [x] **Closes:** #40117 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments - Ported the same fix applied to CmdPal to ensure the process is terminated gracefully instead of being killed: https://github.com/microsoft/PowerToys/pull/39589 - Attempt to cleanup Preview Handlers and close the relative process ## Validation Steps Performed Tested manually: - Preview through some Excel and Word files - Close Peek window - Excel and Word processes are closed --- .../ShellPreviewHandlerPreviewer.cs | 16 ++++++++++++++-- src/modules/peek/Peek.UI/PeekXAML/App.xaml.cs | 2 ++ .../peek/Peek.UI/PeekXAML/MainWindow.xaml.cs | 3 +++ src/modules/peek/peek/dllmain.cpp | 16 +++++++++++----- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/modules/peek/Peek.FilePreviewer/Previewers/ShellPreviewHandlerPreviewer/ShellPreviewHandlerPreviewer.cs b/src/modules/peek/Peek.FilePreviewer/Previewers/ShellPreviewHandlerPreviewer/ShellPreviewHandlerPreviewer.cs index 608ce78f3f..7dbcada42d 100644 --- a/src/modules/peek/Peek.FilePreviewer/Previewers/ShellPreviewHandlerPreviewer/ShellPreviewHandlerPreviewer.cs +++ b/src/modules/peek/Peek.FilePreviewer/Previewers/ShellPreviewHandlerPreviewer/ShellPreviewHandlerPreviewer.cs @@ -90,14 +90,12 @@ namespace Peek.FilePreviewer.Previewers unsafe { // This runs the preview handler in a separate process (prevhost.exe) - // TODO: Figure out how to get it to run in a low integrity level if (!HandlerFactories.TryGetValue(clsid, out var factory)) { var hr = PInvoke_FilePreviewer.CoGetClassObject(clsid, CLSCTX.CLSCTX_LOCAL_SERVER, null, typeof(IClassFactory).GUID, out object pFactory); Marshal.ThrowExceptionForHR(hr); // Storing the factory in memory helps makes the handlers load faster - // TODO: Maybe free them after some inactivity or when Peek quits? factory = (IClassFactory)pFactory; factory.LockServer(true); HandlerFactories.AddOrUpdate(clsid, factory, (_, _) => factory); @@ -213,6 +211,20 @@ namespace Peek.FilePreviewer.Previewers return !string.IsNullOrEmpty(GetPreviewHandlerGuid(item.Extension)); } + public static void ReleaseHandlerFactories() + { + foreach (var factory in HandlerFactories.Values) + { + try + { + Marshal.FinalReleaseComObject(factory); + } + catch + { + } + } + } + private static string? GetPreviewHandlerGuid(string fileExt) { const string PreviewHandlerKeyPath = "shellex\\{8895b1c6-b41f-4c1c-a562-0d564250836f}"; diff --git a/src/modules/peek/Peek.UI/PeekXAML/App.xaml.cs b/src/modules/peek/Peek.UI/PeekXAML/App.xaml.cs index 9b753b3224..9bd66e380f 100644 --- a/src/modules/peek/Peek.UI/PeekXAML/App.xaml.cs +++ b/src/modules/peek/Peek.UI/PeekXAML/App.xaml.cs @@ -12,6 +12,7 @@ using Microsoft.UI.Xaml; using Peek.Common; using Peek.FilePreviewer; using Peek.FilePreviewer.Models; +using Peek.FilePreviewer.Previewers; using Peek.UI.Native; using Peek.UI.Telemetry.Events; using Peek.UI.Views; @@ -111,6 +112,7 @@ namespace Peek.UI NativeEventWaiter.WaitForEventLoop(Constants.ShowPeekEvent(), OnPeekHotkey); NativeEventWaiter.WaitForEventLoop(Constants.TerminatePeekEvent(), () => { + ShellPreviewHandlerPreviewer.ReleaseHandlerFactories(); EtwTrace?.Dispose(); Environment.Exit(0); }); diff --git a/src/modules/peek/Peek.UI/PeekXAML/MainWindow.xaml.cs b/src/modules/peek/Peek.UI/PeekXAML/MainWindow.xaml.cs index 3ce9a82bf9..4edad9a807 100644 --- a/src/modules/peek/Peek.UI/PeekXAML/MainWindow.xaml.cs +++ b/src/modules/peek/Peek.UI/PeekXAML/MainWindow.xaml.cs @@ -15,6 +15,7 @@ using Microsoft.UI.Xaml.Input; using Peek.Common.Constants; using Peek.Common.Extensions; using Peek.FilePreviewer.Models; +using Peek.FilePreviewer.Previewers; using Peek.UI.Extensions; using Peek.UI.Helpers; using Peek.UI.Telemetry.Events; @@ -204,6 +205,8 @@ namespace Peek.UI ViewModel.ScalingFactor = 1; this.Content.KeyUp -= Content_KeyUp; + + ShellPreviewHandlerPreviewer.ReleaseHandlerFactories(); } /// diff --git a/src/modules/peek/peek/dllmain.cpp b/src/modules/peek/peek/dllmain.cpp index 5fd4da8039..4c3da5d999 100644 --- a/src/modules/peek/peek/dllmain.cpp +++ b/src/modules/peek/peek/dllmain.cpp @@ -405,13 +405,19 @@ public: { ResetEvent(m_hInvokeEvent); SetEvent(m_hTerminateEvent); - WaitForSingleObject(m_hProcess, 1500); - auto result = TerminateProcess(m_hProcess, 1); - if (result == 0) + + HANDLE hProcess = OpenProcess(SYNCHRONIZE | PROCESS_TERMINATE, FALSE, m_processPid); + if (WaitForSingleObject(hProcess, 1500) == WAIT_TIMEOUT) { - int error = GetLastError(); - Logger::trace("Couldn't terminate the process. Last error: {}", error); + auto result = TerminateProcess(hProcess, 1); + if (result == 0) + { + int error = GetLastError(); + Logger::trace("Couldn't terminate the process. Last error: {}", error); + } } + + CloseHandle(hProcess); CloseHandle(m_hProcess); m_hProcess = 0; m_processPid = 0;