Skip to content

Enable TaskHost Callbacks by default #13579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JanProvaznik merged 2 commits into from
Apr 21, 2026
+5 −188
Merged

Enable TaskHost Callbacks by default #13579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcd087c
Bump PacketVersion to 4, remove MSBUILDENABLETASKHOSTCALLBACKS env var
JanProvaznik Apr 20, 2026
ef00126
Apply suggestion from @JanProvaznik
JanProvaznik Apr 21, 2026
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
179 changes: 1 addition & 178 deletions src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
Expand Down Expand Up @@ -38,7 +38,6 @@ public TaskHostCallback_Tests(ITestOutputHelper output)
public void IsRunningMultipleNodes_WorksWithExplicitTaskHostFactory(int maxNodeCount, bool expectedResult)
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");

string projectContents = $@"
<Project>
Expand Down Expand Up @@ -70,7 +69,6 @@ public void IsRunningMultipleNodes_WorksWithExplicitTaskHostFactory(int maxNodeC
public void IsRunningMultipleNodes_WorksWhenAutoEjectedInMultiThreadedMode(int maxNodeCount, bool expectedResult)
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");
string testDir = env.CreateFolder().Path;

// IsRunningMultipleNodesTask lacks MSBuildMultiThreadableTask attribute, so it's auto-ejected to TaskHost in MT mode
Expand Down Expand Up @@ -107,41 +105,6 @@ public void IsRunningMultipleNodes_WorksWhenAutoEjectedInMultiThreadedMode(int m
logger.FullLog.ShouldContain($"IsRunningMultipleNodes = {expectedResult}");
}

/// <summary>
/// Verifies that accessing IsRunningMultipleNodes when callbacks are disabled
/// logs error MSB5022 (BuildEngineCallbacksInTaskHostUnsupported).
/// This preserves the pre-callback behavior where unsupported IBuildEngine
/// methods in TaskHost log an error.
/// </summary>
[Fact]
public void IsRunningMultipleNodes_LogsErrorWhenCallbacksNotSupported()
{
using TestEnvironment env = TestEnvironment.Create(_output);

// Explicitly do NOT set MSBUILDENABLETASKHOSTCALLBACKS -- callbacks should be disabled
string projectContents = $@"
<Project>
<UsingTask TaskName=""{nameof(IsRunningMultipleNodesTask)}"" AssemblyFile=""{typeof(IsRunningMultipleNodesTask).Assembly.Location}"" TaskFactory=""TaskHostFactory"" />
<Target Name=""Test"">
<{nameof(IsRunningMultipleNodesTask)}>
<Output PropertyName=""Result"" TaskParameter=""IsRunningMultipleNodes"" />
</{nameof(IsRunningMultipleNodesTask)}>
</Target>
</Project>";

TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents);
ProjectInstance projectInstance = new(project.ProjectFile);

var logger = new MockLogger(_output);
BuildResult buildResult = BuildManager.DefaultBuildManager.Build(
new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] },
new BuildRequestData(projectInstance, targetsToBuild: ["Test"]));

// MSB5022 error should be logged -- the callback was not forwarded
logger.ErrorCount.ShouldBeGreaterThan(0);
logger.FullLog.ShouldContain("MSB5022");
}

/// <summary>
/// Verifies RequestCores callback works when task is explicitly run in TaskHost via TaskHostFactory.
/// The first RequestCores call should always return at least 1 (the implicit core).
Expand All @@ -150,7 +113,6 @@ public void IsRunningMultipleNodes_LogsErrorWhenCallbacksNotSupported()
public void RequestCores_WorksWithExplicitTaskHostFactory()
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");

string projectContents = $@"
<Project>
Expand Down Expand Up @@ -182,7 +144,6 @@ public void RequestCores_WorksWithExplicitTaskHostFactory()
public void RequestAndReleaseCores_WorksWithExplicitTaskHostFactory()
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");

string projectContents = $@"
<Project>
Expand Down Expand Up @@ -215,7 +176,6 @@ public void RequestAndReleaseCores_WorksWithExplicitTaskHostFactory()
public void RequestCores_WorksWhenAutoEjectedInMultiThreadedMode()
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");
string testDir = env.CreateFolder().Path;

// RequestCoresTask lacks MSBuildMultiThreadableTask attribute, so it's auto-ejected to TaskHost in MT mode
Expand Down Expand Up @@ -358,96 +318,6 @@ public void GlobalProperties_UseBuildLevelWhenChangeWaveDisabled()
logger.FullLog.ShouldContain("GlobalPropertyCount = 0");
}

/// <summary>
/// Regression test for https://github.com/dotnet/msbuild/issues/13333
/// When callbacks are not supported (cross-version OOP TaskHost), RequestCores must
/// throw NotImplementedException (not log a build error and return 0).
/// Real callers (MonoAOTCompiler, EmccCompile, ILStrip, EmitBundleBase) catch this
/// exception and fall back to their own parallelism estimate. The previous behavior
/// of logging BuildErrorEventArgs caused the build to fail silently.
/// </summary>
[Fact]
public void RequestCores_ThrowsNotImplementedWhenCallbacksNotSupported()
{
using TestEnvironment env = TestEnvironment.Create(_output);

// Explicitly do NOT set MSBUILDENABLETASKHOSTCALLBACKS -- callbacks should be disabled.
// Use RequestCoresWithFallbackTask which catches NotImplementedException like real callers do.
string projectContents = $@"
<Project>
<UsingTask TaskName=""{nameof(RequestCoresWithFallbackTask)}"" AssemblyFile=""{typeof(RequestCoresWithFallbackTask).Assembly.Location}"" TaskFactory=""TaskHostFactory"" />
<Target Name=""Test"">
<{nameof(RequestCoresWithFallbackTask)} CoreCount=""4"">
<Output PropertyName=""GrantedResult"" TaskParameter=""GrantedCores"" />
<Output PropertyName=""FellBack"" TaskParameter=""UsedFallback"" />
</{nameof(RequestCoresWithFallbackTask)}>
</Target>
</Project>";

TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents);
ProjectInstance projectInstance = new(project.ProjectFile);

var logger = new MockLogger(_output);
BuildResult buildResult = BuildManager.DefaultBuildManager.Build(
new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] },
new BuildRequestData(projectInstance, targetsToBuild: ["Test"]));

// Build must succeed -- the task catches NotImplementedException and falls back.
buildResult.OverallResult.ShouldBe(BuildResultCode.Success);

// No errors should be logged -- NotImplementedException is caught by the task, not by MSBuild.
logger.ErrorCount.ShouldBe(0);

// The task should have used its fallback path (NotImplementedException was thrown).
logger.FullLog.ShouldContain("RequestCores threw NotImplementedException, using fallback");

// GrantedCores should be the task's own fallback (CoreCount), not 0.
logger.FullLog.ShouldContain("GrantedCores = 4");
}

/// <summary>
/// Regression test for https://github.com/dotnet/msbuild/issues/13333
/// When callbacks are not supported, the full caller pattern (RequestCores with catch,
/// then skip ReleaseCores) must work. This matches MonoAOTCompiler/EmccCompile/ILStrip:
/// try { cores = be9.RequestCores(N); }
/// catch (NotImplementedException) { be9 = null; }
/// finally { be9?.ReleaseCores(cores); }
/// ReleaseCores must NOT be called when the fallback fires (be9 is nulled).
/// </summary>
[Fact]
public void RequestAndReleaseCores_FallbackSkipsReleaseWhenCallbacksNotSupported()
{
using TestEnvironment env = TestEnvironment.Create(_output);

// Explicitly do NOT set MSBUILDENABLETASKHOSTCALLBACKS -- callbacks should be disabled
string projectContents = $@"
<Project>
<UsingTask TaskName=""{nameof(RequestCoresWithFallbackTask)}"" AssemblyFile=""{typeof(RequestCoresWithFallbackTask).Assembly.Location}"" TaskFactory=""TaskHostFactory"" />
<Target Name=""Test"">
<{nameof(RequestCoresWithFallbackTask)} CoreCount=""4"" ReleaseAfter=""true"">
<Output PropertyName=""GrantedResult"" TaskParameter=""GrantedCores"" />
<Output PropertyName=""FellBack"" TaskParameter=""UsedFallback"" />
</{nameof(RequestCoresWithFallbackTask)}>
</Target>
</Project>";

TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents);
ProjectInstance projectInstance = new(project.ProjectFile);

var logger = new MockLogger(_output);
BuildResult buildResult = BuildManager.DefaultBuildManager.Build(
new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] },
new BuildRequestData(projectInstance, targetsToBuild: ["Test"]));

// Build must succeed.
buildResult.OverallResult.ShouldBe(BuildResultCode.Success);
logger.ErrorCount.ShouldBe(0);

// Fallback fired -- ReleaseCores should have been skipped (nulled in catch).
logger.FullLog.ShouldContain("RequestCores threw NotImplementedException, using fallback");
logger.FullLog.ShouldNotContain("ReleaseCores(");
}

/// <summary>
/// Verifies BuildProjectFile callback works when task is explicitly run in TaskHost via TaskHostFactory.
/// The child project should build and the task should return success.
Expand All @@ -456,7 +326,6 @@ public void RequestAndReleaseCores_FallbackSkipsReleaseWhenCallbacksNotSupported
public void BuildProjectFile_WorksWithExplicitTaskHostFactory()
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");

string childProject = env.CreateFile("Child.proj", """
<Project>
Expand Down Expand Up @@ -496,7 +365,6 @@ public void BuildProjectFile_WorksWithExplicitTaskHostFactory()
public void BuildProjectFile_ForwardsGlobalProperties()
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");

string childProject = env.CreateFile("Child.proj", """
<Project>
Expand Down Expand Up @@ -535,7 +403,6 @@ public void BuildProjectFile_ForwardsGlobalProperties()
public void BuildProjectFile_ReturnsTargetOutputs()
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");

string childProject = env.CreateFile("Child.proj", """
<Project>
Expand Down Expand Up @@ -583,7 +450,6 @@ public void BuildProjectFile_ReturnsTargetOutputs()
public void BuildProjectFile_ChildFailure_ReturnsFalse()
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");

string childProject = env.CreateFile("Child.proj", """
<Project>
Expand Down Expand Up @@ -623,7 +489,6 @@ public void BuildProjectFile_ChildFailure_ReturnsFalse()
public void BuildProjectFile_WorksWhenAutoEjectedInMultiThreadedMode()
{
using TestEnvironment env = TestEnvironment.Create(_output);
env.SetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS", "1");
string testDir = env.CreateFolder().Path;

string childProject = Path.Combine(testDir, "Child.proj");
Expand Down Expand Up @@ -663,47 +528,5 @@ public void BuildProjectFile_WorksWhenAutoEjectedInMultiThreadedMode()
logger.FullLog.ShouldContain("external task host");
logger.FullLog.ShouldContain("ChildBuiltInMT");
}

/// <summary>
/// Verifies that BuildProjectFile when callbacks are disabled logs error MSB5022.
/// </summary>
[Fact]
public void BuildProjectFile_LogsErrorWhenCallbacksNotSupported()
{
using TestEnvironment env = TestEnvironment.Create(_output);

string childProject = env.CreateFile("Child.proj", """
<Project>
<Target Name="Build">
<Message Text="ShouldNotRun" Importance="high" />
</Target>
</Project>
""").Path;

// Explicitly do NOT set MSBUILDENABLETASKHOSTCALLBACKS
string projectContents = $@"
<Project>
<UsingTask TaskName=""{nameof(BuildProjectFileTask)}"" AssemblyFile=""{typeof(BuildProjectFileTask).Assembly.Location}"" TaskFactory=""TaskHostFactory"" />
<Target Name=""Test"">
<{nameof(BuildProjectFileTask)} ProjectFile=""{childProject}"" Targets=""Build"">
<Output PropertyName=""Result"" TaskParameter=""BuildSucceeded"" />
</{nameof(BuildProjectFileTask)}>
</Target>
</Project>";

TransientTestProjectWithFiles project = env.CreateTestProjectWithFiles(projectContents);
ProjectInstance projectInstance = new(project.ProjectFile);

var logger = new MockLogger(_output);
BuildResult buildResult = BuildManager.DefaultBuildManager.Build(
new BuildParameters { MaxNodeCount = 4, EnableNodeReuse = false, Loggers = [logger] },
new BuildRequestData(projectInstance, targetsToBuild: ["Test"]));

// MSB5022 error should be logged
logger.ErrorCount.ShouldBeGreaterThan(0);
logger.FullLog.ShouldContain("MSB5022");
// Child should not have been built
logger.FullLog.ShouldNotContain("ShouldNotRun");
}
}
}
6 changes: 0 additions & 6 deletions src/Framework/Traits.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,6 @@ public Traits()
/// TODO: Replace with command line flag when feature is completed. The environment variable is intented to avoid exposing the flag early.
public readonly bool EnableRarNode = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBuildRarNode"));

/// <summary>
/// Enable IBuildEngine callbacks in the TaskHost process.
/// Temporary escape hatch until all callback stages are complete and PacketVersion is bumped to 3.
/// </summary>
public readonly bool EnableTaskHostCallbacks = Environment.GetEnvironmentVariable("MSBUILDENABLETASKHOSTCALLBACKS") == "1";

/// <summary>
/// Name of environment variables used to enable MSBuild server.
/// </summary>
Expand Down
5 changes: 2 additions & 3 deletions src/MSBuild/OutOfProcTaskHostNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,14 @@ private readonly AsyncLocal<TaskExecutionContext> _currentTaskContext

/// <summary>
/// Minimum packet version required for IBuildEngine callback support.
/// When all callback stages are complete, PacketVersion will be bumped to this value.
/// </summary>
private const byte CallbacksMinPacketVersion = 4;

/// <summary>
/// Whether the owning worker node supports IBuildEngine callbacks.
/// True if the worker node's packet version is high enough, or if the feature is force-enabled via env var.
/// True if the worker node's packet version is high enough.
/// </summary>
private bool CallbacksSupported => _parentPacketVersion >= CallbacksMinPacketVersion || Traits.Instance.EnableTaskHostCallbacks;
private bool CallbacksSupported => _parentPacketVersion >= CallbacksMinPacketVersion;
Comment thread
JanProvaznik marked this conversation as resolved.

/// <summary>
/// Gets the effective configuration for the current task thread.
Expand Down
3 changes: 2 additions & 1 deletion src/Shared/INodePacket.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,11 +339,12 @@ internal static class NodePacketTypeExtensions
/// 1: .NET task host support.
/// 2: Added support for translating/reading HostServices, ProjectFile, TargetName in TaskHostConfiguration.
/// 3: Added App Host support.
/// 4: Added IsRunningMultipleNodes, Request/ReleaseCores, BuildProjectFile callbacks support for OOP TaskHost.
///
/// When incrementing this version, ensure compatibility with existing
/// task hosts and update the corresponding deserialization logic.
/// </summary>
public const byte PacketVersion = 3;
public const byte PacketVersion = 4;

// Flag bits in upper 2 bits
private const byte ExtendedHeaderFlag = 0x40; // Bit 6: 01000000
Expand Down
Loading