From 84296b0d89a3a9548f3e2788c326f0b67622a0fe Mon Sep 17 00:00:00 2001 From: Nathan Gill Date: Sun, 8 Jun 2025 18:06:01 +0100 Subject: [PATCH] [CmdPal] Remove redundant flag to prevent resource leak (#39865) ## Summary of the Pull Request Remove a redundant flag in the thumbnail helper that leads to a resource leak. ## PR Checklist - [x] **Closes:** #39824 - [ ] **Communication:** I've discussed this with core contributors already. If 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 Remove the `SHGFI_ICON` flag from the call to `NativeMethods.SHGetFileInfo`. This flag opens a handle to the icon (`hIcon`) as well as filling the index (`iIcon`) ([docs](https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shgetfileinfow#shgfi_icon-0x000000100)). This handle stored in `shinfo.hIcon` is never used by following code, and is not freed as suggested by the documentation ([docs](https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shgetfileinfow#remarks)). The `SHGFI_ICON` flag also fills `shinfo.iIcon`, which is used later. However, this is also filled when passing the flag `SHGFI_SYSICONINDEX`, which we already pass. Therefore, the `SHGFI_ICON` flag is redundant here, and has been removed. Thanks to @Androvald for finding and suggesting the fix. --- .../ThumbnailHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ThumbnailHelper.cs b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ThumbnailHelper.cs index cf7c5c2073..e06dbfcb22 100644 --- a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ThumbnailHelper.cs +++ b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ThumbnailHelper.cs @@ -123,7 +123,7 @@ public class ThumbnailHelper private static nint GetLargestIcon(string path) { var shinfo = default(NativeMethods.SHFILEINFO); - NativeMethods.SHGetFileInfo(path, 0, ref shinfo, (uint)Marshal.SizeOf(shinfo), SHGFI_ICON | SHGFI_SYSICONINDEX); + NativeMethods.SHGetFileInfo(path, 0, ref shinfo, (uint)Marshal.SizeOf(shinfo), SHGFI_SYSICONINDEX); var hIcon = IntPtr.Zero; var iID_IImageList = new Guid("46EB5926-582E-4017-9FDF-E8998DAA0950");