diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs index 30af960b986..6aab5035b1c 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.IO; @@ -22,13 +23,6 @@ namespace Microsoft.Build.BackEnd /// internal class NodeProviderOutOfProcTaskHost : NodeProviderOutOfProcBase, INodeProvider, INodePacketFactory, INodePacketHandler { - /// - /// The maximum number of nodes that this provider supports. Should - /// always be equivalent to the number of different TaskHostContexts - /// that exist. - /// - private const int MaxNodeCount = 4; - /// /// Store the path for MSBuild / MSBuildTaskHost so that we don't have to keep recalculating it. /// @@ -87,8 +81,14 @@ internal class NodeProviderOutOfProcTaskHost : NodeProviderOutOfProcBase, INodeP /// /// A mapping of all the nodes managed by this provider. /// - private Dictionary _nodeContexts; + private ConcurrentDictionary _nodeContexts; + /// + /// The next node id to assign to a node. + /// + private int _nextNodeId = 1; + + private LockType _nextNodeIdLock = new LockType(); /// /// A mapping of all of the INodePacketFactories wrapped by this provider. /// @@ -135,7 +135,7 @@ public int AvailableNodes { get { - return MaxNodeCount - _nodeContexts.Count; + return int.MaxValue; } } @@ -175,19 +175,9 @@ public IList CreateNodes(int nextNodeId, INodePacketFactory packetFact /// The packet to send. public void SendData(int nodeId, INodePacket packet) { - throw new NotImplementedException("Use the other overload of SendData instead"); - } - - /// - /// Sends data to the specified node. - /// - /// The node to which data shall be sent. - /// The packet to send. - public void SendData(HandshakeOptions hostContext, INodePacket packet) - { - ErrorUtilities.VerifyThrow(_nodeContexts.ContainsKey(hostContext), "Invalid host context specified: {0}.", hostContext.ToString()); + ErrorUtilities.VerifyThrow(_nodeContexts.ContainsKey(nodeId), "Invalid host context specified: {0}.", nodeId); - SendData(_nodeContexts[hostContext], packet); + SendData(_nodeContexts[nodeId], packet); } /// @@ -199,10 +189,7 @@ public void ShutdownConnectedNodes(bool enableReuse) // Send the build completion message to the nodes, causing them to shutdown or reset. List contextsToShutDown; - lock (_nodeContexts) - { - contextsToShutDown = new List(_nodeContexts.Values); - } + contextsToShutDown = new List(_nodeContexts.Values); ShutdownConnectedNodes(contextsToShutDown, enableReuse); @@ -227,7 +214,7 @@ public void ShutdownAllNodes() public void InitializeComponent(IBuildComponentHost host) { this.ComponentHost = host; - _nodeContexts = new Dictionary(); + _nodeContexts = new ConcurrentDictionary(); _nodeIdToPacketFactory = new Dictionary(); _nodeIdToPacketHandler = new Dictionary(); _activeNodes = new HashSet(); @@ -592,24 +579,23 @@ internal bool AcquireAndSetUpHost( INodePacketFactory factory, INodePacketHandler handler, TaskHostConfiguration configuration, - Dictionary taskHostParameters) + Dictionary taskHostParameters, + out int nodeId) { bool nodeCreationSucceeded; - if (!_nodeContexts.ContainsKey(hostContext)) - { - nodeCreationSucceeded = CreateNode(hostContext, factory, handler, configuration, taskHostParameters); - } - else + + lock (_nextNodeIdLock) { - // node already exists, so "creation" automatically succeeded - nodeCreationSucceeded = true; + nodeId = _nextNodeId++; } + nodeCreationSucceeded = CreateNode(hostContext, nodeId, factory, handler, configuration, taskHostParameters); + if (nodeCreationSucceeded) { - NodeContext context = _nodeContexts[hostContext]; - _nodeIdToPacketFactory[(int)hostContext] = factory; - _nodeIdToPacketHandler[(int)hostContext] = handler; + NodeContext context = _nodeContexts[nodeId]; + _nodeIdToPacketFactory[nodeId] = factory; + _nodeIdToPacketHandler[nodeId] = handler; // Configure the node. context.SendData(configuration); @@ -622,33 +608,26 @@ internal bool AcquireAndSetUpHost( /// /// Expected to be called when TaskHostTask is done with host of the given context. /// - internal void DisconnectFromHost(HandshakeOptions hostContext) + internal void DisconnectFromHost(int nodeId) { - ErrorUtilities.VerifyThrow(_nodeIdToPacketFactory.ContainsKey((int)hostContext) && _nodeIdToPacketHandler.ContainsKey((int)hostContext), "Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice?"); + ErrorUtilities.VerifyThrow(_nodeIdToPacketFactory.ContainsKey(nodeId) && _nodeIdToPacketHandler.ContainsKey(nodeId), "Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice?"); - _nodeIdToPacketFactory.Remove((int)hostContext); - _nodeIdToPacketHandler.Remove((int)hostContext); + _nodeIdToPacketFactory.Remove(nodeId); + _nodeIdToPacketHandler.Remove(nodeId); } /// /// Instantiates a new MSBuild or MSBuildTaskHost process acting as a child node. /// - internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factory, INodePacketHandler handler, TaskHostConfiguration configuration, Dictionary taskHostParameters) + internal bool CreateNode(HandshakeOptions hostContext, int nextNodeId, INodePacketFactory factory, INodePacketHandler handler, TaskHostConfiguration configuration, Dictionary taskHostParameters) { ErrorUtilities.VerifyThrowArgumentNull(factory); - ErrorUtilities.VerifyThrow(!_nodeIdToPacketFactory.ContainsKey((int)hostContext), "We should not already have a factory for this context! Did we forget to call DisconnectFromHost somewhere?"); - - if (AvailableNodes <= 0) - { - ErrorUtilities.ThrowInternalError("All allowable nodes already created ({0}).", _nodeContexts.Count); - return false; - } + ErrorUtilities.VerifyThrow(!_nodeIdToPacketFactory.ContainsKey(nextNodeId), "We should not already have a factory for this nodeId! Did we forget to call DisconnectFromHost somewhere?"); // 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} "; IList nodeContexts; - int nodeId = (int)hostContext; // Handle .NET task host context #if NETFRAMEWORK @@ -664,7 +643,7 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor nodeContexts = GetNodes( runtimeHostPath, string.Format(commandLineArgsPlaceholder, Path.Combine(msbuildAssemblyPath, Constants.MSBuildAssemblyName), NodeReuseIsEnabled(hostContext), ComponentHost.BuildParameters.LowPriority), - nodeId, + nextNodeId, this, handshake, NodeContextCreated, @@ -688,7 +667,7 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor nodeContexts = GetNodes( msbuildLocation, string.Format(commandLineArgsPlaceholder, string.Empty, NodeReuseIsEnabled(hostContext), ComponentHost.BuildParameters.LowPriority), - nodeId, + nextNodeId, this, new Handshake(hostContext), NodeContextCreated, @@ -714,7 +693,7 @@ bool NodeReuseIsEnabled(HandshakeOptions hostContext) /// private void NodeContextCreated(NodeContext context) { - _nodeContexts[(HandshakeOptions)context.NodeId] = context; + _nodeContexts[context.NodeId] = context; // Start the asynchronous read. context.BeginAsyncPacketRead(); @@ -731,10 +710,7 @@ private void NodeContextCreated(NodeContext context) /// private void NodeContextTerminated(int nodeId) { - lock (_nodeContexts) - { - _nodeContexts.Remove((HandshakeOptions)nodeId); - } + _nodeContexts.TryRemove(nodeId, out _); // May also be removed by unnatural termination, so don't assume it's there lock (_activeNodes) diff --git a/src/Build/Instance/TaskFactories/TaskHostTask.cs b/src/Build/Instance/TaskFactories/TaskHostTask.cs index be7a41298e9..14100d4d497 100644 --- a/src/Build/Instance/TaskFactories/TaskHostTask.cs +++ b/src/Build/Instance/TaskFactories/TaskHostTask.cs @@ -91,7 +91,9 @@ internal class TaskHostTask : IGeneratedTask, ICancelableTask, INodePacketFactor /// The task host context of the task host we're launching -- used to /// communicate with the task host. /// - private HandshakeOptions _requiredContext = HandshakeOptions.None; + private HandshakeOptions _requiredHandshakeOptions = HandshakeOptions.None; + + private int _requiredNodeId = -1; /// /// True if currently connected to the task host; false otherwise. @@ -251,7 +253,7 @@ public void Cancel() { if (_taskHostProvider != null && _connectedToTaskHost) { - _taskHostProvider.SendData(_requiredContext, new TaskHostTaskCancelled()); + _taskHostProvider.SendData(_requiredNodeId, new TaskHostTaskCancelled()); } } @@ -304,14 +306,14 @@ public bool Execute() { lock (_taskHostLock) { - _requiredContext = CommunicationsUtilities.GetHandshakeOptions( + _requiredHandshakeOptions = 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); + _connectedToTaskHost = _taskHostProvider.AcquireAndSetUpHost(_requiredHandshakeOptions, this, this, hostConfiguration, _taskHostParameters, out _requiredNodeId); } if (_connectedToTaskHost) @@ -340,23 +342,23 @@ public bool Execute() { lock (_taskHostLock) { - _taskHostProvider.DisconnectFromHost(_requiredContext); + _taskHostProvider.DisconnectFromHost(_requiredNodeId); _connectedToTaskHost = false; } } } else { - LogErrorUnableToCreateTaskHost(_requiredContext, runtime, architecture, null); + LogErrorUnableToCreateTaskHost(_requiredHandshakeOptions, runtime, architecture, null); } } catch (BuildAbortedException) { - LogErrorUnableToCreateTaskHost(_requiredContext, runtime, architecture, null); + LogErrorUnableToCreateTaskHost(_requiredHandshakeOptions, runtime, architecture, null); } catch (NodeFailedToLaunchException e) { - LogErrorUnableToCreateTaskHost(_requiredContext, runtime, architecture, e); + LogErrorUnableToCreateTaskHost(_requiredHandshakeOptions, runtime, architecture, e); } return _taskExecutionSucceeded;