Skip to content

Commit

Permalink
Use all MSBuild binding redirects in NuGet.Frameworks AppDomain (#9634)
Browse files Browse the repository at this point in the history
### Context

The secondary AppDomain hosting `NuGet.Frameworks` was set up with a binding redirect for `Microsoft.Build` but not its dependencies. Additionally, `MSBuild.exe` loader policy was set to `MultiDomainHost`, making it possible to share only Framework assemblies between AppDomains, but still loading MSBuild assemblies domain specific. These issues were resulting in NGEN rejections at run-time.

### Changes Made

Made the `GenerateAppDomainConfig` target use all binding redirects from `MSBuild.exe.config` and switched to `MultiDomain` loader policy. Also pluralized the relevant identifiers (redirect -> redirects).

### Testing

Experimental VS insertion with PerfDDRITs & Speedometer runs.

Co-authored-by: Rainer Sigwald <[email protected]>
  • Loading branch information
ladipro and rainersigwald authored Jan 19, 2024
1 parent 6e97308 commit 0932b43
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 12 deletions.
4 changes: 3 additions & 1 deletion documentation/NETFramework-NGEN.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ right binding redirects, allowing MSBuild to use `Assembly.Load` and get the nat

This approach has some small startup cost (building the config, creating AppDomain & a `MarshalByRefObject`) and a small run-time overhead
of cross-domain calls. The former is orders of magnitude smaller that the startup hit of JITting and the latter is negligible as long as
the types moved across the AppDomain boundary do not require expensive marshaling.
the types moved across the AppDomain boundary do not require expensive marshaling. Additionally, the requirement to execute code in multiple
AppDomains necessitates the use of `LoaderOptimization.MultiDomain` for loading all assemblies domain-neutral. This may come with run-time
cost for certain code patterns, although none has been measured in MSBuild scenarios.

## Task assemblies

Expand Down
16 changes: 9 additions & 7 deletions src/Build/Microsoft.Build.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -679,22 +679,24 @@
<NuGetFrameworkWrapperRedirects_FilePath>$(IntermediateOutputPath)NuGetFrameworkWrapper.redirects.cs</NuGetFrameworkWrapperRedirects_FilePath>
</PropertyGroup>

<!-- Extract binding redirects for Microsoft.Build from MSBuild.exe.config into a source file -->
<!-- Extract binding redirects for MSBuild and dependencies from MSBuild.exe.config into a source file.
This allows us to create secondary AppDomains with the same redirects at run-time, see
https://github.com/dotnet/msbuild/blob/main/documentation/NETFramework-NGEN.md#nugetframeworks -->
<Target Name="GenerateAppDomainConfig"
Inputs="..\MSBuild\app.config;..\MSBuild\app.amd64.config"
Inputs="..\MSBuild\app.config;..\MSBuild\app.amd64.config;$(MSBuildThisFileFullPath)"
Outputs="$(NuGetFrameworkWrapperRedirects_FilePath)"
BeforeTargets="CoreCompile"
Condition="'$(FeatureAppDomain)' == 'true'">
<PropertyGroup>
<BindingRedirectNamespace>&lt;Namespace Prefix='ns' Uri='urn:schemas-microsoft-com:asm.v1' /&gt;</BindingRedirectNamespace>
<BindingRedirectXPath>/configuration/runtime/ns:assemblyBinding/ns:dependentAssembly[ns:assemblyIdentity/@name='Microsoft.Build']</BindingRedirectXPath>
<BindingRedirectXPath>/configuration/runtime/ns:assemblyBinding/*</BindingRedirectXPath>
</PropertyGroup>

<XmlPeek XmlInputPath="..\MSBuild\app.config" Query="$(BindingRedirectXPath)" Namespaces="$(BindingRedirectNamespace)">
<Output TaskParameter="Result" ItemName="BindingRedirect32" />
<Output TaskParameter="Result" ItemName="BindingRedirects32" />
</XmlPeek>
<XmlPeek XmlInputPath="..\MSBuild\app.amd64.config" Query="$(BindingRedirectXPath)" Namespaces="$(BindingRedirectNamespace)">
<Output TaskParameter="Result" ItemName="BindingRedirect64" />
<Output TaskParameter="Result" ItemName="BindingRedirects64" />
</XmlPeek>

<PropertyGroup>
Expand All @@ -707,8 +709,8 @@ namespace Microsoft.Build.Evaluation%3B;
[System.CodeDom.Compiler.GeneratedCode("GenerateAppDomainConfig", "1.0")]
internal sealed partial class NuGetFrameworkWrapper
{
private const string _bindingRedirect32 = """;@(BindingRedirect32);"""%3B;
private const string _bindingRedirect64 = """;@(BindingRedirect64);"""%3B;
private const string _bindingRedirects32 = """;@(BindingRedirects32);"""%3B;
private const string _bindingRedirects64 = """;@(BindingRedirects64);"""%3B;
}
]]>
</NuGetFrameworkWrapperRedirects_Content>
Expand Down
7 changes: 6 additions & 1 deletion src/Build/Utilities/NuGetFrameworkWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ public string FilterTargetFrameworks(string incoming, string filter)
/// </summary>
public override object InitializeLifetimeService() => null;

/// <summary>
/// Creates <see cref="AppDomainSetup"/> suitable for loading Microsoft.Build, NuGet.Frameworks, and dependencies.
/// See https://github.com/dotnet/msbuild/blob/main/documentation/NETFramework-NGEN.md#nugetframeworks for the motivation
/// to use a separate AppDomain.
/// </summary>
private static AppDomainSetup CreateAppDomainSetup(AssemblyName assemblyName, string assemblyPath)
{
byte[] publicKeyToken = assemblyName.GetPublicKeyToken();
Expand All @@ -176,7 +181,7 @@ private static AppDomainSetup CreateAppDomainSetup(AssemblyName assemblyName, st
<DisableFXClosureWalk enabled="true" />
<DeferFXClosureWalk enabled="true" />
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
{(Environment.Is64BitProcess ? _bindingRedirect64 : _bindingRedirect32)}
{(Environment.Is64BitProcess ? _bindingRedirects64 : _bindingRedirects32)}
<dependentAssembly>
<assemblyIdentity name="{NuGetFrameworksAssemblyName}" publicKeyToken="{publicKeyTokenString}" culture="{assemblyName.CultureName}" />
<codeBase version="{assemblyName.Version}" href="{assemblyPath}" />
Expand Down
6 changes: 3 additions & 3 deletions src/MSBuild/XMake.cs
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,14 @@ private static void HandleConfigurationException(Exception ex)
/// MSBuild no longer runs any arbitrary code (tasks or loggers) on the main thread, so it never needs the
/// main thread to be in an STA. Accordingly, to avoid ambiguity, we explicitly use the [MTAThread] attribute.
/// This doesn't actually do any work unless COM interop occurs for some reason.
/// We use the MultiDomainHost loader policy because we may create secondary AppDomains and need NGEN images
/// for Framework / GACed assemblies to be loaded domain neutral so their native images can be used.
/// We use the MultiDomain loader policy because we may create secondary AppDomains and need NGEN images
/// for our as well as Framework assemblies to be loaded domain neutral so their native images can be used.
/// See <see cref="NuGetFrameworkWrapper"/>.
/// </remarks>
/// <returns>0 on success, 1 on failure</returns>
[MTAThread]
#if FEATURE_APPDOMAIN
[LoaderOptimization(LoaderOptimization.MultiDomainHost)]
[LoaderOptimization(LoaderOptimization.MultiDomain)]
#endif
#pragma warning disable SA1111, SA1009 // Closing parenthesis should be on line of last parameter
public static int Main(
Expand Down

0 comments on commit 0932b43

Please sign in to comment.