Fix TryGetShellSizedIcon: replace unsafe SHGFI_ICON try/catch with safe SYSICONINDEX+image list

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/6310db0b-d586-4c56-bcd6-57dbb112fa7c

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

src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/ThumbnailHelper.cs

@@ -61,12 +61,10 @@ public static class ThumbnailHelper
     // these are windows constants and mangling them is goofy
 #pragma warning disable SA1310 // Field names should not contain underscore
 #pragma warning disable SA1306 // Field names should begin with lower-case letter
-    private const uint SHGFI_ICON = 0x000000100;
-    private const uint SHGFI_LARGEICON = 0x000000000;
-    private const uint SHGFI_SHELLICONSIZE = 0x000000004;
     private const uint SHGFI_SYSICONINDEX = 0x000004000;
     private const uint SHGFI_PIDL = 0x000000008;
-    private const int SHIL_JUMBO = 4;
+    private const int SHIL_LARGE = 0;   // 32×32 px system large-icon list
+    private const int SHIL_JUMBO = 4;   // 256×256 px jumbo-icon list
     private const int ILD_TRANSPARENT = 1;
 #pragma warning restore SA1306 // Field names should begin with lower-case letter
 #pragma warning restore SA1310 // Field names should not contain underscore
@@ -112,17 +110,18 @@ public static class ThumbnailHelper
             {
                 hIcon = GetLargestIcon(pidl);
             }
-
-            if (hIcon == 0)
+            else
             {
-                var shinfo = default(NativeMethods.SHFILEINFO);
-                var fileInfoResult = NativeMethods.SHGetFileInfo(pidl, 0, ref shinfo, (uint)Marshal.SizeOf(shinfo), SHGFI_ICON | SHGFI_SHELLICONSIZE | SHGFI_LARGEICON | SHGFI_PIDL);
-                if (fileInfoResult != IntPtr.Zero && shinfo.hIcon != IntPtr.Zero)
-                {
-                    hIcon = shinfo.hIcon;
-                }
+                hIcon = GetNormalSizeIcon(pidl);
             }
 
+            // Both GetLargestIcon and GetNormalSizeIcon use SHGFI_SYSICONINDEX|SHGFI_PIDL (which
+            // does NOT trigger IShellIconOverlayIdentifier handlers) to obtain the icon index, then
+            // retrieve the icon from the system image list. This avoids the SHGFI_ICON path that
+            // fatally aborts the process in .NET 9 when certain third-party shell extensions (e.g.
+            // PlasticSCM / Unity Version Control) are installed. The crash is a C-runtime
+            // STATUS_STACK_BUFFER_OVERRUN / __fastfail that bypasses all .NET exception handlers
+            // and cannot be caught; it must be prevented by not invoking the unsafe path.
             if (hIcon == 0)
             {
                 return null;
@@ -143,6 +142,26 @@ public static class ThumbnailHelper
         }
     }
 
+    private static nint GetNormalSizeIcon(IntPtr pidl)
+    {
+        var shinfo = default(NativeMethods.SHFILEINFO);
+
+        // SHGFI_SYSICONINDEX with SHGFI_PIDL returns the system image list index without
+        // loading icon overlay handlers. We then retrieve the icon directly from the system
+        // large-icon image list, which is safe even with third-party shell extensions installed.
+        NativeMethods.SHGetFileInfo(pidl, 0, ref shinfo, (uint)Marshal.SizeOf(shinfo), SHGFI_SYSICONINDEX | SHGFI_PIDL);
+
+        var hIcon = IntPtr.Zero;
+        var iID_IImageList = IID_IImageList;
+
+        if (NativeMethods.SHGetImageList(SHIL_LARGE, ref iID_IImageList, out var imageListPtr) == 0 && imageListPtr != IntPtr.Zero)
+        {
+            hIcon = NativeMethods.ImageList_GetIcon(imageListPtr, shinfo.iIcon, ILD_TRANSPARENT);
+        }
+
+        return hIcon;
+    }
+
     private static async Task<IRandomAccessStream?> GetFileIconStreamUsingFilePath(string filePath, bool jumbo)
     {
         nint hIcon = 0;
@@ -154,19 +173,16 @@ public static class ThumbnailHelper
         }
 
         // If we didn't want the JUMBO icon, or didn't find it, fall back to
-        // the normal icon lookup
+        // extracting the icon directly from the file. We intentionally avoid
+        // SHGetFileInfo with SHGFI_ICON here because it invokes shell icon
+        // overlay handlers (IShellIconOverlayIdentifier) which can fatally crash
+        // the process in .NET 9 when certain third-party shell extensions
+        // (e.g. PlasticSCM / Unity Version Control) are installed.
+        // SHDefExtractIconW (used by ExtractIconHandle) only invokes the file
+        // type's IExtractIcon handler and does not load overlay handlers.
         if (hIcon == 0)
         {
-            var shinfo = default(NativeMethods.SHFILEINFO);
-
-            var hr = NativeMethods.SHGetFileInfo(filePath, 0, ref shinfo, (uint)Marshal.SizeOf(shinfo), SHGFI_ICON | SHGFI_SHELLICONSIZE);
-
-            if (hr == 0 || shinfo.hIcon == 0)
-            {
-                return null;
-            }
-
-            hIcon = shinfo.hIcon;
+            hIcon = ExtractIconHandle(filePath, 0, jumbo);
         }
 
         if (hIcon == 0)
@@ -265,7 +281,7 @@ public static class ThumbnailHelper
             return null;
         }
 
-        // if it's and .exe and without a path, let's find on path:
+        // if it's an .exe and without a path, let's find on path:
         if (Path.GetExtension(path).Equals(".exe", StringComparison.OrdinalIgnoreCase) && !Path.IsPathRooted(path))
         {
             var paths = Environment.GetEnvironmentVariable("PATH")?.Split(';') ?? [];

src/modules/launcher/PowerLauncher/Helper/ErrorReporting.cs

@@ -61,7 +61,7 @@ namespace PowerLauncher.Helper
             // Many bug reports because users see the "Report problem UI" after "the" crash with System.Runtime.InteropServices.COMException 0xD0000701 or 0x80263001.
             // However, displaying this "Report problem UI" during WPF crashes, especially when DWM composition is changing, is not ideal; some users reported it hangs for up to a minute before the "Report problem UI" appears.
             // This change modifies the behavior to log the exception instead of showing the "Report problem UI".
-            if (ExceptionHelper.IsRecoverableDwmCompositionException(e))
+            if (ExceptionHelper.IsRecoverableDwmCompositionException(e as System.Runtime.InteropServices.COMException))
             {
                 var logger = LogManager.GetLogger(LoggerName);
                 logger.Error($"From {(isNotUIThread ? "non" : string.Empty)} UI thread's exception: {ExceptionFormatter.FormatException(e)}");

src/modules/launcher/PowerLauncher/Helper/ExceptionHelper.cs

@@ -3,7 +3,6 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Reflection;
 using System.Runtime.InteropServices;
 
 namespace PowerLauncher.Helper
@@ -23,13 +22,6 @@ namespace PowerLauncher.Helper
         /// </summary>
         internal static bool IsRecoverableDwmCompositionException(Exception exception)
         {
-            // Unwrap TargetInvocationException to get the underlying exception, since WPF's internal
-            // theme-change mechanism invokes handlers via reflection which wraps exceptions this way.
-            if (exception is TargetInvocationException && exception.InnerException != null)
-            {
-                exception = exception.InnerException;
-            }
-
             if (exception is not COMException comException)
             {
                 return false;

src/modules/launcher/PowerLauncher/Helper/ThemeManager.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
 using System.Windows;
@@ -159,16 +160,12 @@ namespace PowerLauncher.Helper
 
                     return;
                 }
-                catch (Exception ex) when (ExceptionHelper.IsRecoverableDwmCompositionException(ex))
+                catch (COMException ex) when (ExceptionHelper.IsRecoverableDwmCompositionException(ex))
                 {
-                    var logHResult = (ex is System.Reflection.TargetInvocationException tie && tie.InnerException != null)
-                        ? tie.InnerException.HResult
-                        : ex.HResult;
-
                     switch (attempt)
                     {
                         case 1:
-                            Log.Warn($"Desktop composition is disabled (HRESULT: 0x{logHResult:X}). Scheduling retries for theme update.", typeof(ThemeManager));
+                            Log.Warn($"Desktop composition is disabled (HRESULT: 0x{ex.HResult:X}). Scheduling retries for theme update.", typeof(ThemeManager));
                             delayMs = InitialDelayMs;
                             break;
                         case < maxAttempts:

src/modules/launcher/Wox.Test/ExceptionHelperTest.cs

@@ -1,118 +0,0 @@
-// 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.
-
-using System;
-using System.Diagnostics.CodeAnalysis;
-using System.Reflection;
-using System.Runtime.CompilerServices;
-using System.Runtime.InteropServices;
-using Microsoft.VisualStudio.TestTools.UnitTesting;
-using PowerLauncher.Helper;
-
-namespace Wox.Test;
-
-[TestClass]
-[SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Win32 naming conventions")]
-public class ExceptionHelperTest
-{
-    private const int DWM_E_COMPOSITIONDISABLED = unchecked((int)0x80263001);
-    private const int STATUS_MESSAGE_LOST_HR = unchecked((int)0xD0000701);
-    private const string PresentationFrameworkSource = "PresentationFramework";
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_NullException_ReturnsFalse()
-    {
-        Assert.IsFalse(ExceptionHelper.IsRecoverableDwmCompositionException(null));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_NonCOMException_ReturnsFalse()
-    {
-        var ex = new InvalidOperationException("Test");
-        Assert.IsFalse(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_COMException_CompositionDisabled_ReturnsTrue()
-    {
-        var ex = new COMException("Desktop composition is disabled", DWM_E_COMPOSITIONDISABLED);
-        Assert.IsTrue(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_COMException_OtherHResult_ReturnsFalse()
-    {
-        var ex = new COMException("Some other COM error", unchecked((int)0x80004005));
-        Assert.IsFalse(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_TargetInvocationException_WrappingCompositionDisabled_ReturnsTrue()
-    {
-        var inner = new COMException("Desktop composition is disabled", DWM_E_COMPOSITIONDISABLED);
-        var ex = new TargetInvocationException("Invocation failed", inner);
-        Assert.IsTrue(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_TargetInvocationException_WrappingMessageLostFromPresentationFramework_ReturnsTrue()
-    {
-        var inner = new COMException("Message lost", STATUS_MESSAGE_LOST_HR)
-        {
-            Source = PresentationFrameworkSource,
-        };
-        var ex = new TargetInvocationException("Invocation failed", inner);
-        Assert.IsTrue(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_TargetInvocationException_WrappingDwmCompositionChangedStackTrace_ReturnsTrue()
-    {
-        var inner = CreateDwmCompositionChangedComException();
-        var ex = new TargetInvocationException("Invocation failed", inner);
-        Assert.IsTrue(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_TargetInvocationException_WrappingUnrelatedCOMException_ReturnsFalse()
-    {
-        var inner = new COMException("Unrelated", unchecked((int)0x80004005));
-        var ex = new TargetInvocationException("Invocation failed", inner);
-        Assert.IsFalse(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_TargetInvocationException_WrappingNonCOMException_ReturnsFalse()
-    {
-        var inner = new InvalidOperationException("Not a COM exception");
-        var ex = new TargetInvocationException("Invocation failed", inner);
-        Assert.IsFalse(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    [TestMethod]
-    public void IsRecoverableDwmCompositionException_TargetInvocationException_NullInner_ReturnsFalse()
-    {
-        var ex = new TargetInvocationException("No inner", null);
-        Assert.IsFalse(ExceptionHelper.IsRecoverableDwmCompositionException(ex));
-    }
-
-    private static COMException CreateDwmCompositionChangedComException()
-    {
-        try
-        {
-            ThrowDwmCompositionChangedComException();
-            throw new AssertFailedException("Expected COMException to be thrown.");
-        }
-        catch (COMException ex)
-        {
-            return ex;
-        }
-    }
-
-    [MethodImpl(MethodImplOptions.NoInlining)]
-    private static void ThrowDwmCompositionChangedComException()
-    {
-        throw new COMException("DWM composition changed", unchecked((int)0x80004005));
-    }
-}