Use ConcurrentDictionary in CachingScriptMetadataResolver

[seesharper]

This PR fixes a bug that was caused by multiple threads accessing the DirectReferenceCache and the MissingReferenceCache inside the CachingScriptMetadataResolver

Occasionally, OmniSharp would fail to initialize for csx files with the following output from the OmniSharp log

"Message": "\"System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.\\n  at System.Collections.Generic.Dictionary`2[TKey,TValue].FindEntry (TKey key) [0x00105] in <0523ad94f2e04325802cd231c518c8d9>:0 \\n  at System.Collections.Generic.Dictionary`2[TKey,TValue].ContainsKey (TKey key) [0x00000] in <0523ad94f2e04325802cd231c518c8d9>:0 \\n  at OmniSharp.Script.CachingScriptMetadataResolver.ResolveReference (System.String reference, System.String baseFilePath, Microsoft.CodeAnalysis.MetadataReferenceProperties properties) [0x0000e] in <1574d5aa72bf4503bac5396b2a7b7b1d>:0 \\n  at Microsoft.CodeAnalysis.CommonReferenceManager`2[TCompilation,TAssemblySymbol].ResolveReferenceDirective (System.String reference, Microsoft.CodeAnalysis.Location location, TCompilation compilation) [0x00042] in <4e50cf4237f34233a7c05babbf054a7b>:0 \\n  at Microsoft.CodeAnalysis.CommonReferenceManager`2[TCompilation,TAssemblySymbol].GetCompilationReferences (TCompilation compilation, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.MetadataReference]& references, System.Collections.Generic.IDictionary`2[System.ValueTuple`2[System.String,System.String],Microsoft.CodeAnalysis.MetadataReference]& boundReferenceDirectives, System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.Location]& referenceDirectiveLocations) [0x00093] in <4e50cf4237f34233a7c05babbf054a7b>:0 \\n  at Microsoft.CodeAnalysis.CommonReferenceManager`2[TCompilation,TAssemblySymbol].ResolveMetadataReferences (TCompilation compilation, System.Collections.Generic.Dictionary`2[TKey,TValue] assemblyReferencesBySimpleName, System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.MetadataReference]& references, System.Collections.Generic.IDictionary`2[System.ValueTuple`2[System.String,System.String],Microsoft.CodeAnalysis.MetadataReference]& boundReferenceDirectiveMap, System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.MetadataReference]& boundReferenceDirectives, System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.CommonReferenceManager`2+AssemblyData[TCompilation,TAssemblySymbol]]& assemblies, System.Collections.Immutable.ImmutableArray`1[Microsoft.CodeAnalysis.PEModule]& modules, Microsoft.CodeAnalysis.DiagnosticBag diagnostics) [0x00000] in <4e50cf4237f34233a7c05babbf054a7b>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation+ReferenceManager.CreateAndSetSourceAssemblyFullBind (Microsoft.CodeAnalysis.CSharp.CSharpCompilation compilation) [0x00018] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation+ReferenceManager.CreateSourceAssemblyForCompilation (Microsoft.CodeAnalysis.CSharp.CSharpCompilation compilation) [0x00008] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetBoundReferenceManager () [0x00008] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.get_SourceAssembly () [0x00000] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.get_Assembly () [0x00000] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetAllUnaliasedModules (Microsoft.CodeAnalysis.PooledObjects.ArrayBuilder`1[T] modules) [0x00000] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.get_GlobalNamespace () [0x0000e] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.Imports.FromGlobalUsings (Microsoft.CodeAnalysis.CSharp.CSharpCompilation compilation) [0x00029] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.BindGlobalImports () [0x00000] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at System.Lazy`1[T].ViaFactory (System.Threading.LazyThreadSafetyMode mode) [0x0001c] in <0523ad94f2e04325802cd231c518c8d9>:0 \\n--- End of stack trace from previous location where exception was thrown ---\\n\\n  at System.LazyHelper.ThrowException () [0x00000] in <0523ad94f2e04325802cd231c518c8d9>:0 \\n  at System.Lazy`1[T].CreateValue () [0x0007e] in <0523ad94f2e04325802cd231c518c8d9>:0 \\n  at System.Lazy`1[T].get_Value () [0x0000a] in <0523ad94f2e04325802cd231c518c8d9>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.get_GlobalImports () [0x00000] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetSourceDeclarationDiagnostics (Microsoft.CodeAnalysis.SyntaxTree syntaxTree, System.Nullable`1[T] filterSpanWithinTree, System.Func`4[T1,T2,T3,TResult] locationFilterOpt, System.Threading.CancellationToken cancellationToken) [0x00000] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetDiagnosticsForSyntaxTree (Microsoft.CodeAnalysis.CompilationStage stage, Microsoft.CodeAnalysis.SyntaxTree syntaxTree, System.Nullable`1[T] filterSpanWithinTree, System.Boolean includeEarlierStages, System.Threading.CancellationToken cancellationToken) [0x0008f] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at Microsoft.CodeAnalysis.CSharp.SyntaxTreeSemanticModel.GetDiagnostics (System.Nullable`1[T] span, System.Threading.CancellationToken cancellationToken) [0x00014] in <9a90050056dc4bc086821787fff644c7>:0 \\n  at OmniSharp.Roslyn.CSharp.Workers.Diagnostics.CSharpDiagnosticWorker.GetDiagnosticsForDocument (Microsoft.CodeAnalysis.Document document, System.String projectName) [0x00166] in <6461ce0d9f364c94ba094268e94de20a>:0 \\n  at OmniSharp.Roslyn.CSharp.Workers.Diagnostics.CSharpDiagnosticWorker.GetDiagnostics (System.Collections.Immutable.ImmutableArray`1[T] documentPaths) [0x001fa] in <6461ce0d9f364c94ba094268e94de20a>:0 \\n  at OmniSharp.Roslyn.CSharp.Services.Diagnostics.CodeCheckService.Handle (OmniSharp.Models.CodeCheck.CodeCheckRequest request) [0x00095] in <6461ce0d9f364c94ba094268e94de20a>:0 \\n  at OmniSharp.Endpoint.EndpointHandler`2[TRequest,TResponse].HandleAllRequest (TRequest request, OmniSharp.Protocol.RequestPacket packet) [0x001c1] in <8158084499204c2aa6f4ed70fc564aa2>:0 \\n  at OmniSharp.Endpoint.EndpointHandler`2[TRequest,TResponse].Process (OmniSharp.Protocol.RequestPacket packet, OmniSharp.Endpoint.LanguageModel model, Newtonsoft.Json.Linq.JToken requestObject) [0x002ee] in <8158084499204c2aa6f4ed70fc564aa2>:0 \\n  at OmniSharp.Stdio.Host.HandleRequest (System.String json, Microsoft.Extensions.Logging.ILogger logger) [0x0013b] in <fbfd64cb00e94059948669ed215c3dd9>:0 \""

The fix is implemented by replacing the underlying Dictionary with a ConcurrentDictionary

[filipw]

this type is duplicated for Cake too, could you fix it there too? https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Cake/CachingScriptMetadataResolver.cs

[filipw]

I don't think it's the same thing as before. the old code would only cache non-nulls, whereas the new code would cache them.
In the second case it wouldn't cache empty collections too

[seesharper]

Yeah, that is true, But does it matter? If the _defaultReferenceResolver returns null, it would be null for any subsequent requests too? So we save a call into the _defaultReferenceResolver. Unless that could change between request. If it actually is important we can replace this with TryAdd instead

[seesharper]

Or is it any other reason for this conditional caching? 😀

[filipw]

LGTM thanks

[bjorkstromm]

LGTM