Skip to content

Commit 6772529

Browse files
Make dictionaries concurrent in NodeProvideOutOfProcTaskHost (#12708)
Fixes #12660 ### Context The following assert fails: `/mt /m` sometimes crashes with error: ``` Microsoft.Build.Framework.InternalErrorException: MSB0001: Internal MSBuild Error: Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice? at Microsoft.Build.Shared.ErrorUtilities.ThrowInternalError(String message, Exception innerException, Object[] args) in C:\Users\alinama\work\msbuild\src\Shared\ErrorUtilities.cs:line 69 at Microsoft.Build.Shared.ErrorUtilities.VerifyThrow(Boolean condition, String unformattedMessage) in C:\Users\alinama\work\msbuild\src\Shared\ErrorUtilities.cs:line 195 at Microsoft.Build.BackEnd.NodeProviderOutOfProcTaskHost.DisconnectFromHost(Int32 nodeId) in C:\Users\alinama\work\msbuild\src\Build\BackEnd\Components\Communications\NodeProviderOutOfProcTaskHost.cs:line 607 at Microsoft.Build.BackEnd.TaskHostTask.Execute() in C:\Users\alinama\work\msbuild\src\Build\Instance\TaskFactories\TaskHostTask.cs:line 382 at Microsoft.Build.BackEnd.TaskExecutionHost.Execute() in C:\Users\alinama\work\msbuild\src\Build\BackEnd\TaskExecutionHost\TaskExecutionHost.cs:line 636 ``` ### Changes Made Changed `_nodeIdToPacketFactory` and `_nodeIdToPacketHandler` from `Dictionary` to `ConcurrentDictionary`. ### Testing Manually tested to make sure the error is not there. ### Notes --------- Co-authored-by: Jan Provazník <[email protected]>
1 parent 895abae commit 6772529

File tree

1 file changed

+11
-7
lines changed

1 file changed

+11
-7
lines changed

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,17 @@ internal class NodeProviderOutOfProcTaskHost : NodeProviderOutOfProcBase, INodeP
8686

8787
/// <summary>
8888
/// A mapping of all of the INodePacketFactories wrapped by this provider.
89+
/// Thread-safe to support parallel taskhost creation in /mt mode where multiple thread nodes
90+
/// can simultaneously create their own taskhosts.
8991
/// </summary>
90-
private IDictionary<int, INodePacketFactory> _nodeIdToPacketFactory;
92+
private ConcurrentDictionary<int, INodePacketFactory> _nodeIdToPacketFactory;
9193

9294
/// <summary>
9395
/// A mapping of all of the INodePacketHandlers wrapped by this provider.
96+
/// Thread-safe to support parallel taskhost creation in /mt mode where multiple thread nodes
97+
/// can simultaneously create their own taskhosts.
9498
/// </summary>
95-
private IDictionary<int, INodePacketHandler> _nodeIdToPacketHandler;
99+
private ConcurrentDictionary<int, INodePacketHandler> _nodeIdToPacketHandler;
96100

97101
/// <summary>
98102
/// Keeps track of the set of nodes for which we have not yet received shutdown notification.
@@ -208,8 +212,8 @@ public void InitializeComponent(IBuildComponentHost host)
208212
{
209213
this.ComponentHost = host;
210214
_nodeContexts = new ConcurrentDictionary<int, NodeContext>();
211-
_nodeIdToPacketFactory = new Dictionary<int, INodePacketFactory>();
212-
_nodeIdToPacketHandler = new Dictionary<int, INodePacketHandler>();
215+
_nodeIdToPacketFactory = new ConcurrentDictionary<int, INodePacketFactory>();
216+
_nodeIdToPacketHandler = new ConcurrentDictionary<int, INodePacketHandler>();
213217
_activeNodes = new HashSet<int>();
214218

215219
_noNodesActiveEvent = new ManualResetEvent(true);
@@ -602,10 +606,10 @@ internal bool AcquireAndSetUpHost(
602606
/// </summary>
603607
internal void DisconnectFromHost(int nodeId)
604608
{
605-
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?");
609+
bool successRemoveFactory = _nodeIdToPacketFactory.TryRemove(nodeId, out _);
610+
bool successRemoveHandler = _nodeIdToPacketHandler.TryRemove(nodeId, out _);
606611

607-
_nodeIdToPacketFactory.Remove(nodeId);
608-
_nodeIdToPacketHandler.Remove(nodeId);
612+
ErrorUtilities.VerifyThrow(successRemoveFactory && successRemoveHandler, "Why are we trying to disconnect from a context that we already disconnected from? Did we call DisconnectFromHost twice?");
609613
}
610614

611615
/// <summary>

0 commit comments

Comments
 (0)