Deadlock in PropertyDictionary equality

[rainersigwald]

I was running tests locally when I noticed a hang. I attached a debugger and see that it's two PropertyDictionary instances trying to .Equals against each other:

> Debug.ListThreads
 Index Id     Name                           Location
--------------------------------------------------------------------------------
*1     13400  Worker Thread                  Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance>.this[string].get
 2     15468  Worker Thread                  System.Collections.Concurrent.ConcurrentDictionary<Microsoft.Build.BackEnd.ConfigurationMetadata, object>.TryRemoveInternal
 3     13888  <No Name>                      <no stack frames>
 4     15948  <No Name>                      <no stack frames>
 5     12528  Main Thread                    System.Collections.Concurrent.ConcurrentDictionary<System.__Canon, System.__Canon>.AcquireLocks
 6     8600   Worker Thread                  Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance>.this[string].get
 7     20000  <No Name>                      Xunit.Sdk.MessageBus.ReporterWorker
 8     16780  <No Name>                      <no stack frames>
 9     11244  <No Name>                      <no stack frames>
 10    10444  <No Name>                      <no stack frames>
> Debug.ListCallStack /ShowLineOffset:yes /Thread:1
Callstack for Thread 1 (Thread Id: 13400 (0x3458)):
 Index  Function
--------------------------------------------------------------------------------
 1      Microsoft.Build.dll!Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance>.this[string].get(string name) Line 189
*2      Microsoft.Build.dll!Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance>.Equals(Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance> other) Line 286
 3      Microsoft.Build.dll!Microsoft.Build.BackEnd.ConfigurationMetadata.InternalEquals(Microsoft.Build.BackEnd.ConfigurationMetadata other) Line 165
 4      Microsoft.Build.dll!Microsoft.Build.BackEnd.ConfigurationMetadata.Equals(Microsoft.Build.BackEnd.ConfigurationMetadata other) Line 148
 5      System.Private.CoreLib.dll!System.Collections.Generic.GenericEqualityComparer<Microsoft.Build.BackEnd.ConfigurationMetadata>.Equals(Microsoft.Build.BackEnd.ConfigurationMetadata x, Microsoft.Build.BackEnd.ConfigurationMetadata y)
 6      System.Collections.Concurrent.dll!System.Collections.Concurrent.ConcurrentDictionary<Microsoft.Build.BackEnd.ConfigurationMetadata, object>.TryRemoveInternal(Microsoft.Build.BackEnd.ConfigurationMetadata key, out object value, bool matchValue, object oldValue)
 7      System.Collections.Concurrent.dll!System.Collections.Concurrent.ConcurrentDictionary<Microsoft.Build.BackEnd.ConfigurationMetadata, object>.TryRemove(Microsoft.Build.BackEnd.ConfigurationMetadata key, out object value)
 8      Microsoft.Build.dll!Microsoft.Build.Experimental.Graph.ProjectGraph.FindGraphNodes.AnonymousMethod__1(System.Threading.Tasks.Task _) Line 705
 9      System.Private.CoreLib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state)
 10     System.Private.CoreLib.dll!System.Threading.Tasks.Task.ExecuteWithThreadLocal(ref System.Threading.Tasks.Task currentTaskSlot)
 11     System.Private.CoreLib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch()

> Debug.ListCallStack /ShowLineOffset:yes /Thread:6
Callstack for Thread 6 (Thread Id: 8600 (0x2198)):
 Index  Function
--------------------------------------------------------------------------------
 1      Microsoft.Build.dll!Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance>.this[string].get(string name) Line 189
 2      Microsoft.Build.dll!Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance>.Equals(Microsoft.Build.Collections.PropertyDictionary<Microsoft.Build.Execution.ProjectPropertyInstance> other) Line 286
 3      Microsoft.Build.dll!Microsoft.Build.BackEnd.ConfigurationMetadata.InternalEquals(Microsoft.Build.BackEnd.ConfigurationMetadata other) Line 165
 4      Microsoft.Build.dll!Microsoft.Build.BackEnd.ConfigurationMetadata.Equals(Microsoft.Build.BackEnd.ConfigurationMetadata other) Line 148
 5      System.Private.CoreLib.dll!System.Collections.Generic.GenericEqualityComparer<Microsoft.Build.BackEnd.ConfigurationMetadata>.Equals(Microsoft.Build.BackEnd.ConfigurationMetadata x, Microsoft.Build.BackEnd.ConfigurationMetadata y)
 6      System.Collections.Concurrent.dll!System.Collections.Concurrent.ConcurrentDictionary<Microsoft.Build.BackEnd.ConfigurationMetadata, Microsoft.Build.Experimental.Graph.ProjectGraphNode>.TryAddInternal(Microsoft.Build.BackEnd.ConfigurationMetadata key, int hashcode, Microsoft.Build.Experimental.Graph.ProjectGraphNode value, bool updateIfExists, bool acquireLock, out Microsoft.Build.Experimental.Graph.ProjectGraphNode resultingValue)
 7      System.Collections.Concurrent.dll!System.Collections.Concurrent.ConcurrentDictionary<Microsoft.Build.BackEnd.ConfigurationMetadata, Microsoft.Build.Experimental.Graph.ProjectGraphNode>.this[Microsoft.Build.BackEnd.ConfigurationMetadata].set(Microsoft.Build.BackEnd.ConfigurationMetadata key, Microsoft.Build.Experimental.Graph.ProjectGraphNode value)
 8      Microsoft.Build.dll!Microsoft.Build.Experimental.Graph.ProjectGraph.CreateNewNode(Microsoft.Build.BackEnd.ConfigurationMetadata configurationMetadata, Microsoft.Build.Evaluation.ProjectCollection projectCollection, Microsoft.Build.Experimental.Graph.ProjectGraph.ProjectInstanceFactoryFunc projectInstanceFactory, System.Collections.Concurrent.ConcurrentDictionary<Microsoft.Build.BackEnd.ConfigurationMetadata, Microsoft.Build.Experimental.Graph.ProjectGraphNode> allParsedProjects) Line 647
 9      Microsoft.Build.dll!Microsoft.Build.Experimental.Graph.ProjectGraph.FindGraphNodes.AnonymousMethod__0() Line 676
 10     System.Private.CoreLib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state)
 11     System.Private.CoreLib.dll!System.Threading.Tasks.Task.ExecuteWithThreadLocal(ref System.Threading.Tasks.Task currentTaskSlot)
 12     System.Private.CoreLib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch()

That's a deadlock because .Equals locks this._properties and then uses an indexer into other[key], but the indexer locks this._properties:

https://github.com/microsoft/msbuild/blob/431919a7a99dda21a7a4c0a7fab2726a289f6c4b/src/Build/Collections/PropertyDictionary.cs#L282-L292

https://github.com/microsoft/msbuild/blob/431919a7a99dda21a7a4c0a7fab2726a289f6c4b/src/Build/Collections/PropertyDictionary.cs#L189-L192

Here we're doing a ton of these comparisons to get hashtable lookups into the tasksInProgress dictionary (of ConfigurationMetadata which has a GlobalProperties dictionary) during static graph creation, but I think this could happen in other times/places.