Sidecar fix for CLR2 runtime + respect nodereuse from buildparams

src/Build.UnitTests/BackEnd/ProcessIdTaskSidecar.cs

@@ -9,7 +9,7 @@
 namespace Microsoft.Build.UnitTests
 {
     /// <summary>
-    /// This task was created for https://github.com/dotnet/msbuild/issues/3141
+    /// This task was created for https://github.com/dotnet/msbuild/issues/3141.
     /// </summary>
     public class ProcessIdTaskSidecar : Task
     {

src/Build.UnitTests/BackEnd/TaskHostFactory_Tests.cs

@@ -115,7 +115,7 @@ public void TaskNodesDieAfterBuild(bool taskHostFactorySpecified, bool envVariab
         /// can coexist in the same build and operate independently.
         /// </summary>
         [Fact]
-        public void TransientandSidecarNodeCanCoexist()
+        public void TransientAndSidecarNodeCanCoexist()
         {
             using (TestEnvironment env = TestEnvironment.Create(_output))
             {

src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs

@@ -647,8 +647,6 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor
             // if runtime host path is null it means we don't have MSBuild.dll path resolved and there is no need to include it in the command line arguments.
             string commandLineArgsPlaceholder = "{0} /nologo /nodemode:2 /nodereuse:{1} /low:{2} ";
 
-            bool enableNodeReuse = ComponentHost.BuildParameters.EnableNodeReuse && Handshake.IsHandshakeOptionEnabled(hostContext, HandshakeOptions.NodeReuse);
-
             IList<NodeContext> nodeContexts;
             int nodeId = (int)hostContext;
 
@@ -665,7 +663,7 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor
                 // There is always one task host per host context so we always create just 1 one task host node here.      
                 nodeContexts = GetNodes(
                     runtimeHostPath,
-                    string.Format(commandLineArgsPlaceholder, Path.Combine(msbuildAssemblyPath, Constants.MSBuildAssemblyName), enableNodeReuse, ComponentHost.BuildParameters.LowPriority),
+                    string.Format(commandLineArgsPlaceholder, Path.Combine(msbuildAssemblyPath, Constants.MSBuildAssemblyName), NodeReuseIsEnabled(hostContext), ComponentHost.BuildParameters.LowPriority),
                     nodeId,
                     this,
                     handshake,
@@ -689,7 +687,7 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor
 
             nodeContexts = GetNodes(
                 msbuildLocation,
-                string.Format(commandLineArgsPlaceholder, string.Empty, enableNodeReuse, ComponentHost.BuildParameters.LowPriority),
+                string.Format(commandLineArgsPlaceholder, string.Empty, NodeReuseIsEnabled(hostContext), ComponentHost.BuildParameters.LowPriority),
                 nodeId,
                 this,
                 new Handshake(hostContext),
@@ -698,6 +696,17 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor
                 1);
 
             return nodeContexts.Count == 1;
+
+            // Determines whether node reuse should be enabled for the given host context.
+            // Node reuse allows MSBuild to reuse existing task host processes for better performance,
+            // but is disabled for CLR2 because it uses legacy MSBuildTaskHost.
+            bool NodeReuseIsEnabled(HandshakeOptions hostContext)
+            {
+                bool isCLR2 = Handshake.IsHandshakeOptionEnabled(hostContext, HandshakeOptions.CLR2);
+
+                return Handshake.IsHandshakeOptionEnabled(hostContext, HandshakeOptions.NodeReuse)
+                    && !isCLR2;
+            }
         }
 
         /// <summary>

src/Build/Instance/TaskFactories/AssemblyTaskFactory.cs

@@ -393,7 +393,7 @@ internal ITask CreateTaskInstance(
                     buildComponentHost,
                     mergedParameters,
                     _loadedType,
-                    _isTaskHostFactory
+                    taskHostFactoryExplicitlyRequested: _isTaskHostFactory
 #if FEATURE_APPDOMAIN
                     , appDomainSetup
 #endif

src/Build/Instance/TaskFactories/TaskHostTask.cs

@@ -124,17 +124,15 @@ internal class TaskHostTask : IGeneratedTask, ICancelableTask, INodePacketFactor
         /// </summary>
         private bool _taskExecutionSucceeded = false;
 
-
         /// <summary>
         /// This separates the cause where we force all tasks to run in a task host via environment variables and TaskHostFactory
         /// The difference is that TaskHostFactory requires the TaskHost to be transient i.e. to expire after build.
         /// </summary>
         private bool _taskHostFactoryExplicitlyRequested = false;
 
         /// <summary>
-        /// Constructor
+        /// Constructor.
         /// </summary>
-        ///
 #pragma warning disable SA1111, SA1009 // Closing parenthesis should be on line of last parameter
         public TaskHostTask(
             IElementLocation taskLocation,
@@ -301,7 +299,13 @@ public bool Execute()
             {
                 lock (_taskHostLock)
                 {
-                    _requiredContext = CommunicationsUtilities.GetHandshakeOptions(taskHost: true, nodeReuse: !_taskHostFactoryExplicitlyRequested, taskHostParameters: _taskHostParameters);
+                    _requiredContext = CommunicationsUtilities.GetHandshakeOptions(
+                        taskHost: true,
+
+                        // Determine if we should use node reuse based on build parameters or user preferences (comes from UsingTask element).
+                        // If the user explicitly requested the task host factory, then we always disable node reuse due to the transient nature of task host factory hosts.
+                        nodeReuse: _buildComponentHost.BuildParameters.EnableNodeReuse && !_taskHostFactoryExplicitlyRequested,
+                        taskHostParameters: _taskHostParameters);
                     _connectedToTaskHost = _taskHostProvider.AcquireAndSetUpHost(_requiredContext, this, this, hostConfiguration, _taskHostParameters);
                 }
 

src/MSBuild/XMake.cs

@@ -716,6 +716,9 @@ public static ExitType Execute(
 #endif
 
                 GatherAllSwitches(commandLine, out var switchesFromAutoResponseFile, out var switchesNotFromAutoResponseFile, out _);
+
+                CommunicationsUtilities.Trace($"Command line parameters: {commandLine}");
+
                 bool buildCanBeInvoked = ProcessCommandLineSwitches(
                                             switchesFromAutoResponseFile,
                                             switchesNotFromAutoResponseFile,

src/Shared/CommunicationsUtilities.cs

@@ -185,7 +185,7 @@ internal Handshake(HandshakeOptions nodeType, string predefinedToolsDirectory =
         {
         }
 
-        // Helper method to validate handshake option presense.
+        // Helper method to validate handshake option presence
         internal static bool IsHandshakeOptionEnabled(HandshakeOptions hostContext, HandshakeOptions option) => (hostContext & option) == option;
 
         // Source options of the handshake.
@@ -920,7 +920,8 @@ internal static HandshakeOptions GetHandshakeOptions(
                     break;
             }
 
-            if (nodeReuse)
+            // Node reuse is not supported in CLR2 because it's a legacy runtime.
+            if (nodeReuse && clrVersion != 2)
             {
                 context |= HandshakeOptions.NodeReuse;
             }