Code Quality: Replace raw ABI pointers with ComObject instances for safer disposal#18684
Code Quality: Replace raw ABI pointers with ComObject instances for safer disposal#186840x5bfa wants to merge 1 commit into
Conversation
|
FYI @dongle-the-gadget too |
There was a problem hiding this comment.
Pull request overview
This PR modernizes COM interop across the WinUI app and related projects by moving away from raw ABI pointers / manual Release() patterns toward CsWin32’s newer source-generated COM interfaces, with the intent of improving safety and maintainability (and aligning with the ongoing reliability/performance work tracked in #4180).
Changes:
- Enable CsWin32 COM source generators + marshaling and update manual GUID helpers to value-based
Guidconstants. - Replace many
ComPtr<T>/raw-pointer call sites with generated COM interfaces,outparameters, and a newComHelpersutility for activation/casting/bind-to-handler. - Refactor file-operation progress sink implementation from a manually-built vtable to a
[GeneratedComClass]implementation.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Files.App/ViewModels/UserControls/Widgets/QuickAccessWidgetViewModel.cs | Switch agile reference acquisition/resolution from raw pointers to generated COM interfaces. |
| src/Files.App/ViewModels/UserControls/Previews/ShellPreviewViewModel.cs | Replace D3D/DComp COM pointer plumbing with generated COM interface usage. |
| src/Files.App/Utils/Taskbar/SystemTrayIconWindow.cs | Update WNDPROC delegate alias to managed delegate type in CsWin32 extras. |
| src/Files.App/Utils/Storage/Helpers/SyncRootHelpers.cs | Replace status/quota COM calls with generated COM interfaces + ComHelpers. |
| src/Files.App/Utils/Storage/Helpers/MtpHelpers.cs | Replace raw shell COM pointers/enumeration with generated interfaces. |
| src/Files.App/Utils/Shell/PreviewHandler.cs | Update preview handler activation and initialization interfaces to generated COM types. |
| src/Files.App/Utils/Shell/OpenWithMenu.cs | Replace IContextMenu* lifetime management with generated COM interface usage. |
| src/Files.App/Utils/Shell/LaunchHelper.cs | Update activation manager creation to ComHelpers.CoCreateInstance. |
| src/Files.App/Utils/Shell/ItemStreamHelper.cs | Change helpers to return IShellItem/IStream interfaces instead of IntPtr + manual release. |
| src/Files.App/Utils/Shell/DetectionAndSharingHelper.cs | Replace COM pointer usage with generated COM interface calls. |
| src/Files.App/Services/Windows/WindowsWallpaperService.cs | Switch wallpaper COM activation to ComHelpers and generated interfaces. |
| src/Files.App/Services/Windows/WindowsRecentItemsService.cs | Replace shell folder enumeration + context menu invocation with generated COM interfaces. |
| src/Files.App/Services/Windows/WindowsDialogService.cs | Update file dialogs to use generated COM interfaces and ComHelpers. |
| src/Files.App/Services/Storage/StorageTrashBinService.cs | Replace Recycle Bin/file operation COM pointer usage with generated interfaces. |
| src/Files.App/Data/Items/WindowEx.cs | Update callback delegate aliases to managed delegate types in CsWin32 extras. |
| src/Files.App/Data/Items/WidgetRecentItem.cs | Change ShellItem storage from ComPtr<IShellItem> to IShellItem. |
| src/Files.App.Storage/Windows/WindowsStorable.cs | Change ThisPtr and ContextMenu from raw pointers to generated COM interfaces. |
| src/Files.App.Storage/Windows/WindowsFolder.cs | Update folder wrapper to use generated COM interfaces and managed enumeration. |
| src/Files.App.Storage/Windows/WindowsFile.cs | Update file wrapper to use generated COM interfaces. |
| src/Files.App.Storage/Windows/WindowsBulkOperationsSink.VTable.cs | Remove manually-implemented vtable-based progress sink. |
| src/Files.App.Storage/Windows/WindowsBulkOperationsSink.Methods.cs | Replace sink with [GeneratedComClass] implementation using WeakReference. |
| src/Files.App.Storage/Windows/WindowsBulkOperations.cs | Replace raw IFileOperation*/sink pointers with generated interfaces and managed sink. |
| src/Files.App.Storage/Windows/Managers/WindowsObjectPicker.cs | Update COM activation, IDataObject access, and VARIANT handling to managed marshalling types. |
| src/Files.App.Storage/Windows/Managers/WindowsFolderChangeWatcher.cs | Update WNDPROC delegate alias to managed delegate type in CsWin32 extras. |
| src/Files.App.Storage/Windows/Managers/WindowsDriveManager.cs | Adjust FOLDERID usage to value Guid instead of pointer deref. |
| src/Files.App.Storage/Windows/Managers/TaskbarManager.cs | Replace ComPtr<ITaskbarList3> with generated interface reference. |
| src/Files.App.Storage/Windows/Managers/SystemTrayManager.cs | Update WNDPROC delegate alias to managed delegate type in CsWin32 extras. |
| src/Files.App.Storage/Windows/IWindowsStorable.cs | Update interface to use generated COM interfaces instead of raw pointers. |
| src/Files.App.Storage/Windows/IWindowsFolder.cs | Update interface to use generated COM interfaces instead of raw pointers. |
| src/Files.App.Storage/Windows/Helpers/WindowsStorableHelpers.Shell.cs | Update shell helper extensions to use generated COM interfaces and ComHelpers. |
| src/Files.App.Storage/Windows/Helpers/WindowsStorableHelpers.Icon.cs | Update thumbnail/icon helper COM interactions to generated interfaces. |
| src/Files.App.Server/Program.cs | Update activation factory callback signature to unmanaged interface type. |
| src/Files.App.Server/Helpers.cs | Update GetActivationFactory signature/casts to unmanaged interface type. |
| src/Files.App.CsWin32/NativeMethods.json | Enable marshaling and COM source generators for CsWin32. |
| src/Files.App.CsWin32/ManualGuid.cs | Replace pointer-based GUID accessors with value Guid properties. |
| src/Files.App.CsWin32/IStorageProviderStatusUISourceFactory.cs | Convert to [GeneratedComInterface] with managed signatures. |
| src/Files.App.CsWin32/IStorageProviderStatusUISource.cs | Convert to [GeneratedComInterface] with managed signatures. |
| src/Files.App.CsWin32/IStorageProviderStatusUI.cs | Convert to [GeneratedComInterface] with managed signatures. |
| src/Files.App.CsWin32/IStorageProviderQuotaUI.cs | Convert to [GeneratedComInterface] with managed signatures. |
| src/Files.App.CsWin32/IOpenControlPanel.cs | Convert to [GeneratedComInterface] with managed signatures. |
| src/Files.App.CsWin32/IDetectionAndSharing.cs | Convert to [GeneratedComInterface] with managed signatures. |
| src/Files.App.CsWin32/Files.App.CsWin32.csproj | Run CsWin32 as a build task to keep outputs aligned with generator config. |
| src/Files.App.CsWin32/Extras.cs | Add managed callback delegates and update IDCompositionTarget.SetRoot signature. |
| src/Files.App.CsWin32/ComPtr`1.cs | Remove custom ComPtr<T> implementation. |
| src/Files.App.CsWin32/ComHelpers.cs | Add helper methods for COM activation/casting/bind-to-handler with generated interfaces. |
Comments suppressed due to low confidence (1)
src/Files.App/Utils/Shell/LaunchHelper.cs:28
- LaunchSettings ignores the HRESULT from CoCreateInstance and then uses the null-forgiving operator to call ActivateApplication. If COM activation fails, this will throw NullReferenceException. Handle activation failure explicitly and return early.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Ready for review. |
| [GeneratedComInterface, Guid("BA6295C3-312E-544F-9FD5-1F81B21F3649"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] | ||
| public partial interface IStorageProviderQuotaUI |
There was a problem hiding this comment.
GeneratedComInterface won't work on the IStorageProvider APIs as they are IInspectable, not IUnknown.
There was a problem hiding this comment.
Fixed in the latest commit, please take another look.
| [GeneratedComInterface, Guid("D6B6A758-198D-5B80-977F-5FF73DA33118"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] | ||
| public partial interface IStorageProviderStatusUI |
| [GeneratedComInterface, Guid("A306C249-3D66-5E70-9007-E43DF96051FF"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] | ||
| public partial interface IStorageProviderStatusUISource |
| [GeneratedComInterface, Guid("12E46B74-4E5A-58D1-A62F-0376E8EE7DD8"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] | ||
| public partial interface IStorageProviderStatusUISourceFactory |
| public static Guid IID_IStorageProviderStatusUISourceFactory { get; } = new("12E46B74-4E5A-58D1-A62F-0376E8EE7DD8"); | ||
|
|
||
| public static Guid IID_IContextMenu { get; } = new("000214E4-0000-0000-C000-000000000046"); | ||
| public static Guid IID_IEnumShellItems { get; } = new("70629033-E363-4A28-A567-0DB78006E6D7"); | ||
| public static Guid IID_IShellItem { get; } = new("43826D1E-E718-42EE-BC55-A1E261C37BFE"); | ||
| public static Guid IID_IShellItem2 { get; } = new("7E9FB0D3-919F-4307-AB2E-9B1860310C93"); | ||
| public static Guid IID_IFileOperation { get; } = new("947AAB5F-0A5C-4C13-B4D6-4BF7836FC9F8"); | ||
| public static Guid IID_IFileOpenDialog { get; } = new("D57C7288-D4AD-4768-BE02-9D969532D960"); | ||
| public static Guid IID_IFileSaveDialog { get; } = new("84BCCD23-5FDE-4CDB-AEA4-AF64B83D78AB"); | ||
| public static Guid IID_IDesktopWallpaper { get; } = new("B92B56A9-8B55-4E14-9A89-0199BBB6F93B"); | ||
| public static Guid IID_IApplicationActivationManager { get; } = new("2E941141-7F97-4756-BA1D-9DECDE894A3D"); | ||
| public static Guid IID_IQueryInfo { get; } = new("00021500-0000-0000-C000-000000000046"); | ||
| public static Guid IID_IShellItemImageFactory { get; } = new("BCC18B79-BA16-442F-80C4-8A59C30C463B"); | ||
| public static Guid IID_IShellLinkW { get; } = new("000214F9-0000-0000-C000-000000000046"); | ||
| public static Guid IID_IShellItemArray { get; } = new("B63EA76D-1F85-456F-A19C-48159EFA858B"); | ||
| public static Guid IID_IExecuteCommand { get; } = new("7F9185B0-CB92-43C5-80A9-92277A4F7B54"); | ||
| public static Guid IID_IObjectWithSelection { get; } = new("1C9CD5BB-98E9-4491-A60F-31AACC72B83C"); | ||
| public static Guid IID_IShellExtInit { get; } = new("000214E8-0000-0000-C000-000000000046"); | ||
| public static Guid IID_IContextMenu2 { get; } = new("000214F4-0000-0000-C000-000000000046"); | ||
| public static Guid IID_IDataObject { get; } = new("0000010E-0000-0000-C000-000000000046"); |
There was a problem hiding this comment.
Nitpicking: using the string overload of Guid constructor here is rather inefficient. Using the int/byte overload would be better.
| public static Guid CLSID_FileOperation { get; } = new("3AD05575-8857-4850-9277-11B85BDB8E09"); | ||
| public static Guid CLSID_FileOpenDialog { get; } = new("DC1C5A9C-E88A-4DDE-A5A1-60F82A20AEF7"); | ||
| public static Guid CLSID_FileSaveDialog { get; } = new("C0B4E2F3-BA21-4773-8DBA-335EC946EB8B"); | ||
| public static Guid CLSID_DesktopWallpaper { get; } = new("C2CF3110-460E-4FC1-B9D0-8A1C0C9CC4BD"); | ||
| public static Guid CLSID_ApplicationActivationManager { get; } = new("45BA127D-10A8-46EA-8AB7-56EA9078943C"); | ||
| public static Guid CLSID_PinToFrequentExecute { get; } = new("B455F46E-E4AF-4035-B0A4-CF18D2F6F28E"); | ||
| public static Guid CLSID_UnPinFromFrequentExecute { get; } = new("EE20EEBA-DF64-4A4E-B7BB-2D1C6B2DFCC1"); | ||
| public static Guid CLSID_NewMenu { get; } = new("D969A300-E7FF-11d0-A93B-00A0C90F2719"); | ||
| public static Guid CLSID_OpenWithMenu { get; } = new("09799AFB-AD67-11D1-ABCD-00C04FC30936"); | ||
| public static Guid CLSID_DetectionAndSharing { get; } = new("1FDA955B-61FF-11DA-978C-0008744FAAB7"); | ||
| public static Guid CLSID_OpenControlPanel { get; } = new("06622D85-6856-4460-8DE1-A81921B41C4B"); |
| public static Guid BHID_SFUIObject { get; } = new("3981E225-F559-11D3-8E3A-00C04F6837D5"); | ||
| public static Guid BHID_EnumItems { get; } = new("94F60519-2850-4924-AA5A-D15E84868039"); | ||
| public static Guid BHID_DataObject { get; } = new("B8C0BD9F-ED24-455C-83E6-D5390C4FE8C4"); |
| public static Guid FOLDERID_RecycleBinFolder { get; } = new("B7534046-3ECB-4C18-BE4E-64CD4CB7D6AC"); | ||
| public static Guid FOLDERID_ComputerFolder { get; } = new("0AC0837C-BBF8-452A-850D-79D08E667CA7"); |
Resolved / Related Issues
We've been using raw ABI pointes, having us dispose COM instances via IUnknown::Release manually. This makes us a lot harder to manage the lifetime of each instance because normally managed instances are disposed automatically by GC and C# developers are not expected to take life cycle of any instance into account. This was because CsWin32 has only supported raw ABI pointers or ComImport, which is AOT unsafe, however, now CsWin32 supports generation of
GeneratedComInterfaceCOM interfaces. COM instances wrapped byComObjectare a managed object, we can use them safely without any trouble.Steps used to test these changes
CC @yair100 @hez2010