CsWin32 follow-up: polyfills, ValueStringBuilder, and additional interop migrations#13705
Conversation
…rop migrations Builds on the initial CsWin32 migration with additional cleanup and migrations: - Add ValueStringBuilder to Framework (adapted from JeremyKuhne/touki, without the interpolated-string handler) and use it in NodeLauncher to build the command line and environment block into pinnable buffers, replacing Marshal.StringToHGlobalUni. - Add Polyfills/ for Framework: IComIID (and supporting helpers) gated by #if !NET so struct-based COM works on net472, plus Span<char>.Replace and ReadOnlySpan<T>.IndexOfAnyExcept polyfills. - Unify RunningObjectTable on the CsWin32 struct-based COM path. - LockCheck: use WIN32_ERROR locals/parameters and RM_APP_* enums; add a FILETIME.ToDateTime() helper and document the type-matching rule. - Migrate additional P/Invokes to CsWin32: CommunicationsUtilities env vars; NodeLauncher CreateProcess; Utilities/LockCheck Restart Manager; UntrustedLocationCheck SHGetKnownFolderPath; Tasks CreateHardLink/MoveFileEx/GetLogicalDrives, GetFileVersion (mscoree), and BootstrapperUtil resource updater APIs. - Remove dead crypt32/advapi32 P/Invokes and the unused CreateProcess declaration from Tasks/NativeMethods. - Add unit tests for ValueStringBuilder, the SpanExtensions polyfill, and the FILETIME helpers.
🔍 Skill Validator Results✅ All checks passed
Summary
Full validator output```text Found 2 skill(s) [cswin32-com] 📊 cswin32-com: 1,788 BPE tokens [chars/4: 1,702] (detailed ✓), 9 sections, 4 code blocks [cswin32-interop] 📊 cswin32-interop: 2,351 BPE tokens [chars/4: 2,263] (detailed ✓), 13 sections, 3 code blocks ✅ All checks passed (2 skill(s)) ``` |
There was a problem hiding this comment.
Pull request overview
This PR is a follow-up to the initial CsWin32 migration, extending struct-based interop usage across MSBuild and adding supporting infrastructure (polyfills + ValueStringBuilder) to improve compatibility and reduce allocations in Windows interop-heavy paths.
Changes:
- Added
ValueStringBuilder(Framework) and Span polyfills, plus unit tests for these helpers and FILETIME conversions. - Migrated additional Win32 P/Invokes to CsWin32 (Restart Manager / ROT / CreateProcess / SHGetKnownFolderPath / env block APIs) and removed now-dead native interop declarations.
- Updated build/project wiring and documentation to reflect the new CsWin32 + COM polyfill approach across TFMs.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Utilities/LockCheck.cs | Migrates Restart Manager usage to CsWin32 and adds typed WIN32_ERROR/RM enums (but introduces a build break when FEATURE_WINDOWSINTEROP is off). |
| src/Tasks/NativeMethods.cs | Removes unused Win32 crypto/process interop and switches several Windows APIs to CsWin32. |
| src/Tasks/BootstrapperUtil/ResourceUpdater.cs | Ports bootstrapper resource updating to CsWin32 and adds an UpdateResource helper. |
| src/Tasks/BootstrapperUtil/NativeMethods.cs | Deletes legacy bootstrapper resource-update P/Invokes. |
| src/Tasks/AssemblyDependency/AssemblyInformation.cs | Migrates mscoree!GetFileVersion usage to CsWin32. |
| src/Framework/Windows/Win32/GeneratedInteropClsCompliance.cs | Adds CLS compliance suppressions for additional CsWin32 COM structs. |
| src/Framework/Windows/Win32/Foundation/FileTimeExtensions.cs | Adds FILETIME → DateTime helpers for local/UTC conversions. |
| src/Framework/Utilities/ValueStringBuilder.cs | Introduces pooled/stack-based string building for interop-friendly, pinnable buffers. |
| src/Framework/Polyfills/SpanExtensions.cs | Adds net472/netstandard Span API polyfills (Replace, IndexOfAnyExcept). |
| src/Framework/Polyfills/IComIIDPolyfills.cs | Adds net472/netstandard COM IID polyfills by attaching IComIID to CsWin32 COM structs via partials. |
| src/Framework/Polyfills/IComIID.cs | Adds the instance-based IComIID polyfill for non-.NET TFMs. |
| src/Framework/NativeMethods.txt | Updates CsWin32 inputs to generate additional APIs/types used by the migrations. |
| src/Framework/Microsoft.Build.Framework.csproj | Adjusts compile-item conditions to include COM helpers and the new polyfill structure correctly. |
| src/Framework/Framework/Windows/Win32/IComIID.cs | Removes the old location of the net472 IComIID polyfill (replaced by Polyfills/). |
| src/Framework/BackEnd/CommunicationsUtilities.cs | Migrates environment block retrieval/freeing to CsWin32. |
| src/Framework.UnitTests/ValueStringBuilder_Tests.cs | Adds unit tests for ValueStringBuilder. |
| src/Framework.UnitTests/SpanExtensions_Tests.cs | Adds unit tests for Span polyfills (non-.NET TFMs only). |
| src/Framework.UnitTests/FileTimeExtensions_Tests.cs | Adds unit tests for FILETIME extension helpers. |
| src/Build/Microsoft.Build.csproj | Removes a now-unneeded native interop source file from compilation. |
| src/Build/Instance/RunningObjectTable.cs | Switches ROT implementation to struct-based COM via CsWin32 and adds richer COM error details. |
| src/Build/BuildCheck/Checks/UntrustedLocationCheck.cs | Migrates SHGetKnownFolderPath to CsWin32 with a safe PWSTR free path. |
| src/Build/BackEnd/Components/Communications/NodeLauncher.cs | Uses ValueStringBuilder to build pinnable command line/env blocks and migrates CreateProcess to CsWin32. |
| src/Build.UnitTests/BackEnd/BuildRequest_Tests.cs | Updates Windows platform attributes to match revised minimum supported versions. |
| .github/skills/cswin32-interop/SKILL.md | Documents enum/type preference and FILETIME conversion helpers for interop work. |
| .github/skills/cswin32-com/SKILL.md | Documents COM struct usage, plus the new IComIID polyfill approach for non-.NET TFMs. |
| .editorconfig | Expands CLS compliance suppression scope to cover the new COM IID polyfill file. |
There was a problem hiding this comment.
Review Summary: CsWin32 COM Interop Migration
This is a well-executed migration from manual P/Invoke to CsWin32-generated types. The PR correctly replaces BackendNativeMethods, Ole32 interop, Crypt32/Advapi32 dead P/Invokes, and Restart Manager manual types with their CsWin32 equivalents. The IComIID polyfill pattern for net472/netstandard2.0 is sound and follows the established winforms approach.
Strengths
- Correctness: The
ValueStringBuilder+fixedpattern forCreateProcessWcorrectly provides a writable, null-terminated buffer — satisfying the Win32 API contract. - Resource cleanup:
ComScope<T>usage inRunningObjectTableproperly manages COM pointer lifetimes for transient objects (monikers, IUnknown results). - Environment block: The
ValueStringBuilder-based approach eliminatesMarshal.StringToHGlobalUni/Marshal.FreeHGlobalwith a cleaner stack-first allocation pattern. - MAKEINTRESOURCE: Correct
(PCWSTR)(char*)intValuepattern for integer resource types. - Behavioral equivalence on Unix: The
CreateNoWindow = redirectStreamssimplification preserves the original semantics sinceredirectStreamswas onlytruewhenCREATENOWINDOWwas set.
Concerns (see inline comments)
- RunningObjectTable finalizer without IDisposable (medium) — Raw COM pointer released only via finalizer; risks crash-on-shutdown if COM is torn down first. Previous RCW had built-in shutdown protection.
- ResourceUpdater silent
return false(low) — Non-Windows path returns false without logging, which may confuse callers checking the return value.
No breaking changes detected
- CLI behavior unchanged
- No new warnings or errors introduced
- Public API surface unaffected
- All conditional compilation guards (
#if FEATURE_WINDOWSINTEROP,#if !NET) correctly preserve behavior on all TFMs
Note
🔒 Integrity filter blocked 1 item
The following item were blocked because they don't meet the GitHub integrity level.
- #13705
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Expert Code Review (on open) for issue #13705 · ● 20.4M
- SpanExtensions.IndexOfAnyExcept: use EqualityComparer<T>.Default for null-safe semantics matching BCL behavior. - RunningObjectTable: marshal both CreateItemMoniker and IRunningObjectTable.GetObject onto an MTA thread together so the cached interface pointer is never accessed cross-apartment from STA callers. - Tasks/NativeMethods.MakeHardLink: provide a deterministic error message in the !FEATURE_WINDOWSINTEROP branch instead of relying on an undefined Win32 last-error. - BootstrapperUtil/ResourceUpdater.UpdateResources: log an explicit error to BuildResults in the !FEATURE_WINDOWSINTEROP branch, and add a Debug.Assert guarding MAKEINTRESOURCE values. - NodeLauncher: add a comment noting that CreateProcess may modify lpCommandLine in place.
- Gate ResourceUpdater.ERROR_SHARING_VIOLATION, StringToByteArray, and the related using directives behind #if FEATURE_WINDOWSINTEROP so they don't trip CA1823/IDE0005/IDE0051 in DotNetBuildSourceOnly builds. - Update the cswin32-interop skill to call out CA1823 and to remind that polyfills referenced only from FEATURE_WINDOWSINTEROP code must be validated against the source build.
The CsWin32-generated ReadOnlySpan<string> wrapper for RmRegisterResources rents arrays from ArrayPool, then iterates the (oversized) rented array calling GCHandle.Free() on uninitialized entries — throwing 'Handle is not initialized.' for any non-empty input. This silently broke LockCheck.GetProcessesLockingFile, which surfaced as Copy_Tests.DoRetryWhenDestinationLocked failing because the locking-process PID was no longer included in the error message. Switch to the lower-level pointer overload, pinning paths via GCHandle in a try/finally that only frees what was allocated.
0.3.275 includes the fix for microsoft/CsWin32#1421 (RmRegisterResources throws InvalidOperationException: Handle is not initialized), which was caused by the generated ReadOnlySpan<string> wrapper iterating the (oversized) ArrayPool-rented array in its finally block and calling GCHandle.Free() on uninitialized entries. Fix landed in microsoft/CsWin32#1405 and shipped in v0.3.228. Reverts the LockCheck workaround from 9eb67be since it's no longer needed, and adapts NativeMethods.GetLogicalCoreCountOnWindows / GetFullPath / LockCheck.RmGetList to the newer CsWin32 generated overload signatures (Span/ref-uint variants).
Continues the CsWin32 struct-based COM migration from dotnet#13705 and dotnet#13746, this time covering the CLR metadata APIs (IMetaDataDispenser / IMetaDataImport2 / IMetaDataAssemblyImport) consumed by AssemblyInformation and ManifestUtil.MetadataReader, plus the ICreateTypeLib path used by RegisterAssembly. New files - src/Tasks/AssemblyDependency/Metadata/ - Metadata.cs folder header / namespace docs - IMetaDataDispenser.cs OpenScope with typed CorOpenFlags - IMetaDataImport2.cs GetCustomAttributeByName + GetPEKind with canonical inherited 3-72 slot map - IMetaDataAssemblyImport.cs GetAssemblyProps / GetAssemblyRefProps / GetFileProps / EnumAssemblyRefs / EnumFiles / GetAssemblyFromScope / CloseEnum - ASSEMBLYMETADATA.cs blittable struct from cor.h - OSINFO.cs blittable struct from cor.h - CorOpenFlags.cs typed [Flags] enum - CorAssemblyFlags.cs typed [Flags] enum - CorTokenType.cs 26 mdt* table-type tags - Tokens.cs MdToken / MdAssembly / MdAssemblyRef / MdFile readonly structs with validating constructors - src/Tasks/TypeLib/ICreateTypeLib.cs struct-based wrapper for RegisterAssembly - src/Tasks.UnitTests/MetadataReader_Tests.cs 12 tests / 24 cross-TFM (PEReader on net10, COM on net472), verifies identity parity for Accessibility.dll Modifications - AssemblyInformation.cs uses MdAssembly / BufferScope<char> / typed CorAssemblyFlags; OpenScope asks for IMetaDataImport2 directly (skips one QI); AllocAsmMeta / FreeAsmMeta eliminated - MetadataReader.cs net472 path migrated from [ComImport] to struct-based COM via AgileComPointer; failure to OpenScope returns null (per-file contract) - RegisterAssembly.cs uses the new ICreateTypeLib struct - Microsoft.Build.Tasks.csproj wildcard <Compile Include> for the Fusion / Metadata / TypeLib folders (FeatureWindowsInterop-gated) - NativeMethods.cs legacy [ComImport] declarations removed; only oleaut32 typelib P/Invokes remain; System.Runtime.Versioning using gated on FEATURE_WINDOWSINTEROP for source-build Skills - .github/skills/cswin32-com/SKILL.md adds 'Strongly-typed handle / token wrappers' with CLR validation table; pair-pointer to cswin32-interop; error- handling-parity guidance; IComIID polyfill heading rename - .github/skills/cswin32-interop/SKILL.md reorganized: Rules / Blittable signatures / Infrastructure / BufferScope<T> / Gotchas; source-build verification note Verified - Normal build: 0 warnings, 0 errors (net472 + net10.0) - Source build: 0 warnings, 0 errors (DotNetBuildSourceOnly=true DotNetBuild=true) - Tests: 24/24 pass on both TFMs
CsWin32 follow-up: CLR metadata + TypeLib interop migration Continues the CsWin32 struct-based COM migration from #13705 and #13746, this time covering the CLR metadata APIs (IMetaDataDispenser / IMetaDataImport2 / IMetaDataAssemblyImport) consumed by AssemblyInformation and ManifestUtil.MetadataReader, plus the ICreateTypeLib path used by RegisterAssembly. New files - src/Tasks/AssemblyDependency/Metadata/ - Metadata.cs folder header / namespace docs - IMetaDataDispenser.cs OpenScope with typed CorOpenFlags - IMetaDataImport2.cs GetCustomAttributeByName + GetPEKind with canonical inherited 3-72 slot map - IMetaDataAssemblyImport.cs GetAssemblyProps / GetAssemblyRefProps / GetFileProps / EnumAssemblyRefs / EnumFiles / GetAssemblyFromScope / CloseEnum - ASSEMBLYMETADATA.cs blittable struct from cor.h - OSINFO.cs blittable struct from cor.h - CorOpenFlags.cs typed [Flags] enum - CorAssemblyFlags.cs typed [Flags] enum - CorTokenType.cs 26 mdt* table-type tags - Tokens.cs MdToken / MdAssembly / MdAssemblyRef / MdFile readonly structs with validating constructors - src/Tasks/TypeLib/ICreateTypeLib.cs struct-based wrapper for RegisterAssembly - src/Tasks.UnitTests/MetadataReader_Tests.cs 12 tests / 24 cross-TFM (PEReader on net10, COM on net472), verifies identity parity for Accessibility.dll Modifications - AssemblyInformation.cs uses MdAssembly / BufferScope<char> / typed CorAssemblyFlags; OpenScope asks for IMetaDataImport2 directly (skips one QI); AllocAsmMeta / FreeAsmMeta eliminated - MetadataReader.cs net472 path migrated from [ComImport] to struct-based COM via AgileComPointer; failure to OpenScope returns null (per-file contract) - RegisterAssembly.cs uses the new ICreateTypeLib struct - Microsoft.Build.Tasks.csproj wildcard <Compile Include> for the Fusion / Metadata / TypeLib folders (FeatureWindowsInterop-gated) - NativeMethods.cs legacy [ComImport] declarations removed; only oleaut32 typelib P/Invokes remain; System.Runtime.Versioning using gated on FEATURE_WINDOWSINTEROP for source-build Skills - .github/skills/cswin32-com/SKILL.md adds 'Strongly-typed handle / token wrappers' with CLR validation table; pair-pointer to cswin32-interop; error- handling-parity guidance; IComIID polyfill heading rename - .github/skills/cswin32-interop/SKILL.md reorganized: Rules / Blittable signatures / Infrastructure / BufferScope<T> / Gotchas; source-build verification note
Builds on the initial CsWin32 migration with additional cleanup and migrations:
Add ValueStringBuilder to Framework (adapted from runtime/WinForms/Touki/etc.) and use it in NodeLauncher to build the command line and environment block into pinnable buffers, replacing Marshal.StringToHGlobalUni.
Add Polyfills/ for Framework: IComIID (and supporting helpers) gated by #if !NET so struct-based COM works on net472, plus Span.Replace and ReadOnlySpan.IndexOfAnyExcept polyfills.
Unify RunningObjectTable on the CsWin32 struct-based COM path.
LockCheck: use WIN32_ERROR locals/parameters and RM_APP_* enums; add a FILETIME.ToDateTime() helper and document the type-matching rule.
Migrate additional P/Invokes to CsWin32: CommunicationsUtilities env vars; NodeLauncher CreateProcess; Utilities/LockCheck Restart Manager; UntrustedLocationCheck SHGetKnownFolderPath; Tasks CreateHardLink/MoveFileEx/GetLogicalDrives, GetFileVersion (mscoree), and BootstrapperUtil resource updater APIs.
Remove dead crypt32/advapi32 P/Invokes and the unused CreateProcess declaration from Tasks/NativeMethods.
Add unit tests for ValueStringBuilder, the SpanExtensions polyfill, and the FILETIME helpers.