Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
271 changes: 271 additions & 0 deletions src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Reflection;
using System.Text.RegularExpressions;
using Microsoft.Build.BackEnd;
using Microsoft.Build.Execution;
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
Expand Down Expand Up @@ -381,7 +384,256 @@ public void ExplicitTaskHostFactory_OverridesRoutingLogic()
logger.FullLog.ShouldContain("TaskWithAttribute executed");
}

[Fact]
public void RequiresTransientTaskHost_ReturnsTrueForRestoreTask()
{
TaskRouter.RequiresTransientTaskHost(typeof(NuGet.Build.Tasks.RestoreTask)).ShouldBeTrue();
}

[Fact]
public void RequiresTransientTaskHost_ReturnsFalseForRegularTask()
{
TaskRouter.RequiresTransientTaskHost(typeof(NonEnlightenedTestTask)).ShouldBeFalse();
TaskRouter.RequiresTransientTaskHost(typeof(AttributeTestTask)).ShouldBeFalse();
}

[Fact]
public void RequiresTransientTaskHost_RoutedToTaskHost_InMultiThreadedMode()
{
string projectContent = $@"
<Project>
<UsingTask TaskName=""RestoreTask"" AssemblyFile=""{Assembly.GetExecutingAssembly().Location}"" />

<Target Name=""TestTarget"">
<RestoreTask />
</Target>
</Project>";

string projectFile = Path.Combine(_testProjectsDir, "RestoreTaskMT.proj");
File.WriteAllText(projectFile, projectContent);

var logger = new MockLogger(_output);
var buildParameters = new BuildParameters
{
MultiThreaded = true,
Loggers = [logger],
DisableInProcNode = false,
EnableNodeReuse = false,
};

var buildRequestData = new BuildRequestData(
projectFile,
new Dictionary<string, string>(),
null,
["TestTarget"],
null);

BuildManager buildManager = BuildManager.DefaultBuildManager;
BuildResult result = buildManager.Build(buildParameters, buildRequestData);

result.OverallResult.ShouldBe(BuildResultCode.Success);
TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "RestoreTask");
logger.FullLog.ShouldContain("RestoreTask executed");
}

[Fact]
public void RequiresTransientTaskHost_RoutedToTaskHost_InServerMode()
{
string projectContent = $@"
<Project>
<UsingTask TaskName=""RestoreTask"" AssemblyFile=""{Assembly.GetExecutingAssembly().Location}"" />

<Target Name=""TestTarget"">
<RestoreTask />
</Target>
</Project>";

string projectFile = Path.Combine(_testProjectsDir, "RestoreTaskServer.proj");
File.WriteAllText(projectFile, projectContent);

var logger = new MockLogger(_output);
var buildParameters = new BuildParameters
{
MultiThreaded = false,
Loggers = [logger],
DisableInProcNode = false,
EnableNodeReuse = false,

// Simulate running under the MSBuild Server. In production this flag is
// set process-wide by OutOfProcServerNode via BuildParameters.MarkProcessAsLongLivedHost.
IsLongLivedHost = true,
};

var buildRequestData = new BuildRequestData(
projectFile,
new Dictionary<string, string>(),
null,
["TestTarget"],
null);

BuildManager buildManager = BuildManager.DefaultBuildManager;
BuildResult result = buildManager.Build(buildParameters, buildRequestData);

result.OverallResult.ShouldBe(BuildResultCode.Success);
TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "RestoreTask");
logger.FullLog.ShouldContain("RestoreTask executed");
}

[Fact]
public void RequiresTransientTaskHost_GetsFreshProcess_OnEachInvocation_InMultiThreadedMode()
{
string projectContent = $@"
<Project>
<UsingTask TaskName=""RestoreTask"" AssemblyFile=""{Assembly.GetExecutingAssembly().Location}"" />

<Target Name=""TestTarget"">
<RestoreTask />
</Target>
</Project>";

string projectFile = Path.Combine(_testProjectsDir, "RestoreTaskFreshProcess.proj");
File.WriteAllText(projectFile, projectContent);

var logger = new MockLogger(_output);
var buildParameters = new BuildParameters
{
MultiThreaded = true,
Loggers = [logger],
DisableInProcNode = false,

// Reuse must stay ON so we test the workaround, not natural process death.
// With reuse off the test cannot distinguish "transient TaskHost (workaround)"
// from "sidecar TaskHost that happened to die between builds".
EnableNodeReuse = true,
};

var buildRequestData = new BuildRequestData(
projectFile,
new Dictionary<string, string>(),
null,
["TestTarget"],
null);

// Two separate Build cycles. The workaround forces nodeReuse=false on the spawned
// TaskHost so it dies at EndBuild, giving the next Build a fresh process.
BuildManager buildManager = BuildManager.DefaultBuildManager;
BuildResult result1 = buildManager.Build(buildParameters, buildRequestData);
BuildResult result2 = buildManager.Build(buildParameters, buildRequestData);

result1.OverallResult.ShouldBe(BuildResultCode.Success);
result2.OverallResult.ShouldBe(BuildResultCode.Success);
TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "RestoreTask");

int[] pids = ExtractReportedPids(logger.FullLog);

pids.Length.ShouldBe(2, $"Expected two RestoreTask invocations to log a PID. Log:{Environment.NewLine}{logger.FullLog}");
pids[0].ShouldNotBe(pids[1], "Each build must spawn a fresh TaskHost so NuGet static state cannot leak across builds.");
pids.ShouldNotContain(Process.GetCurrentProcess().Id, "TaskHost should be out-of-process from the test runner.");
}

[Fact]
public void RequiresTransientTaskHost_GetsFreshProcess_OnEachInvocation_InServerMode()
{
string projectContent = $@"
<Project>
<UsingTask TaskName=""RestoreTask"" AssemblyFile=""{Assembly.GetExecutingAssembly().Location}"" />

<Target Name=""TestTarget"">
<RestoreTask />
</Target>
</Project>";

string projectFile = Path.Combine(_testProjectsDir, "RestoreTaskFreshProcessServer.proj");
File.WriteAllText(projectFile, projectContent);

var logger = new MockLogger(_output);
var buildParameters = new BuildParameters
{
MultiThreaded = false,
Loggers = [logger],
DisableInProcNode = false,

// Reuse must stay ON so we test the workaround, not natural process death.
EnableNodeReuse = true,
Comment thread
OvesN marked this conversation as resolved.

// Simulate running under the MSBuild Server.
IsLongLivedHost = true,
};

var buildRequestData = new BuildRequestData(
projectFile,
new Dictionary<string, string>(),
null,
["TestTarget"],
null);

// Two separate Build cycles. The workaround forces nodeReuse=false on the spawned
// TaskHost so it dies at EndBuild, giving the next Build a fresh process.
BuildManager buildManager = BuildManager.DefaultBuildManager;
BuildResult result1 = buildManager.Build(buildParameters, buildRequestData);
BuildResult result2 = buildManager.Build(buildParameters, buildRequestData);

result1.OverallResult.ShouldBe(BuildResultCode.Success);
result2.OverallResult.ShouldBe(BuildResultCode.Success);
TaskRouterTestHelper.AssertTaskUsedTaskHost(logger, "RestoreTask");

int[] pids = ExtractReportedPids(logger.FullLog);

pids.Length.ShouldBe(2, $"Expected two RestoreTask invocations to log a PID. Log:{Environment.NewLine}{logger.FullLog}");
pids[0].ShouldNotBe(pids[1], "Each build must spawn a fresh TaskHost so NuGet static state cannot leak across builds.");
pids.ShouldNotContain(Process.GetCurrentProcess().Id, "TaskHost should be out-of-process from the test runner.");
}

private static int[] ExtractReportedPids(string log)
{
var pids = new List<int>();
foreach (Match m in Regex.Matches(log, @"RestoreTask executed in PID=(\d+)"))
{
pids.Add(int.Parse(m.Groups[1].Value));
}

return pids.ToArray();
}

[Fact]
public void RequiresTransientTaskHost_RunsInProcess_WhenNoMTOrServer()
{
string projectContent = $@"
<Project>
<UsingTask TaskName=""RestoreTask"" AssemblyFile=""{Assembly.GetExecutingAssembly().Location}"" />

<Target Name=""TestTarget"">
<RestoreTask />
</Target>
</Project>";

string projectFile = Path.Combine(_testProjectsDir, "RestoreTaskNoMT.proj");
File.WriteAllText(projectFile, projectContent);

var logger = new MockLogger(_output);
var buildParameters = new BuildParameters
{
MultiThreaded = false,
Loggers = [logger],
DisableInProcNode = false,
EnableNodeReuse = false,
};

var buildRequestData = new BuildRequestData(
projectFile,
new Dictionary<string, string>(),
null,
["TestTarget"],
null);

BuildManager buildManager = BuildManager.DefaultBuildManager;
BuildResult result = buildManager.Build(buildParameters, buildRequestData);

result.OverallResult.ShouldBe(BuildResultCode.Success);

TaskRouterTestHelper.AssertTaskRanInProcess(logger, "RestoreTask");
logger.FullLog.ShouldContain("RestoreTask executed");
}

private string CreateTestProject(string taskName, string taskClass)
{
Expand Down Expand Up @@ -483,6 +735,25 @@ public override bool Execute()
#endregion
}

// Test task in the NuGet.Build.Tasks namespace to simulate the real RestoreTask for routing tests.
namespace NuGet.Build.Tasks
{
/// <summary>
/// Simulates the NuGet RestoreTask for testing task routing workaround.
/// Has the same full name (NuGet.Build.Tasks.RestoreTask) that TaskRouter checks.
/// </summary>
public class RestoreTask : Task
{
public override bool Execute()
{
// Include the OS PID so tests can verify each invocation runs in a fresh
// TaskHost process
Log.LogMessage(MessageImportance.High, $"RestoreTask executed in PID={Process.GetCurrentProcess().Id}");
return true;
}
}
}

// Custom attribute definition in Microsoft.Build.Framework namespace to match what TaskRouter expects
// TaskRouter looks for attributes with FullName = "Microsoft.Build.Framework.MSBuildMultiThreadableTaskAttribute"
// Since the real attribute is internal in Framework, we define our own test version here
Expand Down
27 changes: 27 additions & 0 deletions src/Build/BackEnd/BuildManager/BuildParameters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ public class BuildParameters : ITranslatable
/// </summary>
private static string s_startupDirectory = NativeMethodsShared.GetCurrentDirectory();

/// <summary>
/// Process-wide flag indicating that the engine is hosted in a long-lived process
/// (e.g., the MSBuild Server) that persists across multiple build invocations.
/// </summary>
private static bool s_isLongLivedHost;

/// <summary>
/// Indicates whether we should warn when a property is uninitialized when it is used.
/// </summary>
Expand Down Expand Up @@ -328,6 +334,7 @@ internal BuildParameters(BuildParameters other, bool resetEnvironment = false)
Question = other.Question;
IsBuildCheckEnabled = other.IsBuildCheckEnabled;
IsTelemetryEnabled = other.IsTelemetryEnabled;
IsLongLivedHost = other.IsLongLivedHost;
ProjectCacheDescriptor = other.ProjectCacheDescriptor;
_enableTargetOutputLogging = other.EnableTargetOutputLogging;
}
Expand Down Expand Up @@ -799,6 +806,25 @@ internal IToolsetProvider ToolsetProvider
/// </summary>
internal bool IsOutOfProc { get; set; }

/// <summary>
/// True when the engine is hosted in a long-lived process (e.g., MSBuild Server)
/// that persists across multiple build invocations. Used to opt tasks whose static
/// singleton state would leak across invocations out of sidecar TaskHost reuse.
/// See https://github.com/dotnet/msbuild/issues/13315.
/// </summary>
internal bool IsLongLivedHost
{
get => _isLongLivedHost;
set => _isLongLivedHost = value;
}

private bool _isLongLivedHost = s_isLongLivedHost;

Comment thread
OvesN marked this conversation as resolved.
/// <summary>
/// Marks the current process as a long-lived host.
/// </summary>
internal static void MarkProcessAsLongLivedHost() => s_isLongLivedHost = true;

/// <nodoc/>
public ProjectLoadSettings ProjectLoadSettings
{
Expand Down Expand Up @@ -978,6 +1004,7 @@ void ITranslatable.Translate(ITranslator translator)
translator.Translate(ref _reportFileAccesses);
translator.Translate(ref _enableTargetOutputLogging);
translator.Translate(ref _multiThreaded);
translator.Translate(ref _isLongLivedHost);

// ProjectRootElementCache is not transmitted.
// ResetCaches is not transmitted.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,11 +631,17 @@ internal bool AcquireAndSetUpHost(
INodePacketFactory factory,
INodePacketHandler handler,
TaskHostConfiguration configuration,
in TaskHostParameters taskHostParameters)
in TaskHostParameters taskHostParameters,
out int hostProcessId,
out bool wasNewlyCreated)
{
hostProcessId = -1;
wasNewlyCreated = false;

bool nodeCreationSucceeded;
if (!_nodeContexts.ContainsKey(nodeKey))
{
wasNewlyCreated = true;
nodeCreationSucceeded = CreateNode(nodeKey, factory, handler, configuration, taskHostParameters);
}
else
Expand All @@ -654,6 +660,16 @@ internal bool AcquireAndSetUpHost(
handlerStack.Push(handler);
}

try
{
hostProcessId = context.Process?.Id ?? -1;
}
catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex))
{
Comment thread
OvesN marked this conversation as resolved.
// Process has already exited or is otherwise inaccessible; PID is unavailable.
hostProcessId = -1;
}

// Configure the node.
context.SendData(configuration);
return true;
Expand Down
Loading