Fix CLR_E_SHIM_RUNTIMELOAD in RAR's IMetaDataDispenser activation

[JeremyKuhne]

Context

Fixes a regression in RAR introduced by #13853 (CsWin32 CLR metadata + TypeLib interop migration), reported by the VC team as VC.Tests.MsBuild.VC.MsBuild.P2PReferences.08 (CLR+CLR+CLR P2P chain): Referenced_C_IJW.dll is missing from Referencing_A_IJW\Debug\ after build.

Root cause

The migration switched AssemblyInformation / MetadataReader from RCW-based COM to struct-based COM through CsWin32 and AgileComPointer, and along the way activates CLSID_CorMetaDataDispenser via raw CoCreateInstance.

CLSID_CorMetaDataDispenser is registered with mscoree.dll (the .NET Framework shim) as its InprocServer32. CoCreateInstance therefore loads the shim, which then has to call LoadLibraryShim to bind a runtime before it can delegate to MetaDataDllGetClassObject.

In hosts that did not enter the CLR via mscoree's startup path — notably the VC cppxplatdev test harness, which embeds MSBuild in-process via BuildManager from a non-mscoree-rooted process — the shim's bound-runtime state is uninitialized and LoadLibraryShim fails with CLR_E_SHIM_RUNTIMELOAD (0x80131700, "Failed to load the runtime"). RAR catches that as a DependencyResolutionException and silently drops every transitive dependency for the failing reference. End-to-end symptom is the missing Referenced_C_IJW.dll copy.

Standalone msbuild.exe cannot reproduce because the msbuild.exe process enters the CLR via the standard mscoree startup path, so the shim is bound.

Fix

Activate the dispenser by calling clr.dll's exported DllGetClassObjectInternal directly via a new ComClassFactory.TryCreateFromModule overload, bypassing the shim entirely. This is exactly what the CLR's own managed-COM activation does internally for CLSIDs on its IsClrHostedLegacyComObject list. The approach is AOT-friendly (no Type.GetTypeFromCLSID, no Activator, no RCW), so .NET 10+ source-generated COM can use it unchanged.

ComClassFactory

• Two public TryCreateFromModule overloads (standard DllGetClassObject and caller-specified export name).
• Public ClrDllGetClassObjectInternalExportName constant with documentation of the shim mechanism.
• Single delegate* unmanaged[Stdcall]<...> function-pointer path serves both net472 and .NET Core.

Callers

• AssemblyInformation and MetadataReader now use TryCreateFromModule("clr.dll", CLSID, ClrDllGetClassObjectInternalExportName, ...).
• Both retain the previous [PreserveSig] tolerance for GetAssemblyFromScope and GetPEKind failures (the legacy RCW code silently ignored those HRs).
• Pointer release moved to DisposeUnmanagedResources so the finalizer path still revokes GIT cookies if the user forgets to dispose.

Other

• NativeMethods.txt: add GetModuleHandle (kept for completeness — LoadLibrary, FreeLibrary, GetProcAddress were already present).
• New net472-only AssemblyInformation_Tests covering both file-mapping lifetime (delete/overwrite after Dispose, including a repeated open-close stress loop) and dispenser activation.

Validation

• Local unit tests: 8/8 new + 12/12 existing MetadataReader_Tests pass on net472 x86.
• End-to-end: built Microsoft.Build.Framework.dll + Microsoft.Build.Tasks.Core.dll deployed into a local VS18 install, ran the failing Project.vcxproj directly with msbuild.exe against the actual VC P2PReferences asset — both Referenced_B_IJW.dll and Referenced_C_IJW.dll now land in Referencing_A_IJW\Debug\.
• Build clean, 0 warnings/0 errors.

Cc / fyi @rainersigwald @JaynieBai @YuliiaKovalova — needs the VC team to re-run P2PReferences.08 with these binaries to confirm in their CI.

[Copilot]

Pull request overview

This PR fixes a .NET Framework RAR regression when activating CLSID_CorMetaDataDispenser in hosts that embed MSBuild without going through the mscoree.dll shim startup path (triggering CLR_E_SHIM_RUNTIMELOAD). It does so by bypassing the shim and activating the metadata dispenser via clr.dll’s internal class-object export, then adds targeted net472-only regression tests around AssemblyInformation lifetime/activation.

Changes:

• Update AssemblyInformation and ManifestUtil.MetadataReader (net472 path) to activate IMetaDataDispenser via ComClassFactory.TryCreateFromModule("clr.dll", ..., "DllGetClassObjectInternal", ...) instead of raw CoCreateInstance.
• Extend ComClassFactory with module-export-based activation helpers and document the mscoree-shim failure mechanism.
• Add new net472-only AssemblyInformation_Tests covering mapping lifetime and activation sanity.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/Tasks/ManifestUtil/MetadataReader.cs	Switch net472 COM activation to module-export path and preserve legacy HR-tolerance behavior.
src/Tasks/AssemblyDependency/AssemblyInformation.cs	Same activation change; move COM pointer cleanup to unmanaged dispose path to cover finalizer scenarios.
src/Tasks.UnitTests/AssemblyDependency/AssemblyInformation_Tests.cs	New net472-only regression tests for mapping lifetime and activation sanity.
src/Framework/Windows/Win32/System/Com/ComClassFactory.cs	Add TryCreateFromModule overloads and CLR-specific export constant/docs.
src/Framework/NativeMethods.txt	Add GetModuleHandle to the Win32 import list.

[github-actions]

Review of PR #13899 — Fix CLR_E_SHIM_RUNTIMELOAD regression in embedded-host scenarios

The fix correctly identifies the root cause: raw CoCreateInstance on CLSID_CorMetaDataDispenser delegates to mscoree.dll, whose LoadLibraryShim fails (0x80131700) in native-embedded hosts where the shim's startup state isn't initialized. Calling DllGetClassObjectInternal directly on the already-loaded clr.dll bypasses the shim entirely and reproduces what the CLR's own managed-COM activation path does internally. The approach is sound.

The HRESULT-tolerance changes for GetAssemblyFromScope and GetPEKind correctly restore the legacy [PreserveSig] contract that was inadvertently tightened by #13853.

No BLOCKING or MAJOR issues found.

#	Dimension	Verdict
22	Correctness	🟡 1 MODERATE — DisposeUnmanagedResources + managed Dispose is safe but comment is incomplete
24	Security	🟡 1 MODERATE — LoadLibrary DLL search order; GetModuleHandle was added to NativeMethods.txt but unused
3	Performance	🟡 1 MODERATE — LoadLibrary+GetProcAddress+DllGetClassObject per construction; no caching
14	Naming/NIT	🔵 3 NITs — GetModuleHandle dead entry, machine variable, public const on internal class

✅ 20/24 dimensions clean.

• Correctness — DisposeUnmanagedResources comment should explain why calling AgileComPointer.Dispose() from the finalizer path is safe (CLR field-reachability guarantee + Interlocked.Exchange guard in AgileComPointer)
• Security/Correctness — GetModuleHandle was added to NativeMethods.txt but LoadLibrary (with DLL search order) is used instead; GetModuleHandle is the right call for a module that must already be loaded
• Performance — LoadLibrary + GetProcAddress + DllGetClassObjectInternal on every AssemblyInformation construction; cache at least the FARPROC

Note

🔒 Integrity filter blocked 2 items

The following items were blocked because they don't meet the GitHub integrity level.

• #13899 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #13899 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 | none

Generated by Expert Code Review (on open) for issue #13899 · ● 3.7M

[rainersigwald]

Bypassing macOS since the queues are long and this is a Windows-only fix.