Fix a deadlock in the analyzer driver

Primary issue is that we currently try to force complete and decode SuppressMessageAttributes in the analyzer callbacks to report a diagnostic. We need to decode these attributes to apply source suppressions to reported diagnostics. However, force completing attributes can lead to us binding more symbols, and subsequently generating more symbol declared events and attempting to callback into the analyzer again.

Fix is to decode the suppress message attributes as a post pass in the analyzer driver and avoid any symbol force completion during analyzer callbacks to report diagnostics.

Fixes #4858

src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs

@@ -373,7 +373,7 @@ private int RunCore(TextWriter consoleOutput, ErrorLogger errorLogger, Cancellat
                     Action<Diagnostic> addExceptionDiagnostic = diagnostic => analyzerExceptionDiagnostics.Add(diagnostic);
                     var analyzerOptions = new AnalyzerOptions(ImmutableArray<AdditionalText>.CastUp(additionalTextFiles));
                     analyzerDriver = AnalyzerDriver.CreateAndAttachToCompilation(compilation, analyzers, analyzerOptions, analyzerManager, addExceptionDiagnostic, Arguments.ReportAnalyzer, out compilation, analyzerCts.Token);
-                    getAnalyzerDiagnostics = () => analyzerDriver.GetDiagnosticsAsync().Result;
+                    getAnalyzerDiagnostics = () => analyzerDriver.GetDiagnosticsAsync(compilation).Result;
                 }
 
                 // Print the diagnostics produced during the parsing stage and exit if there were any errors.

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisResult.cs

@@ -48,14 +48,14 @@ public TimeSpan GetAnalyzerExecutionTime(DiagnosticAnalyzer analyzer)
             }
         }
 
-        public void StoreAnalysisResult(AnalysisScope analysisScope, AnalyzerDriver driver)
+        public void StoreAnalysisResult(AnalysisScope analysisScope, AnalyzerDriver driver, Compilation compilation)
         {
             foreach (var analyzer in analysisScope.Analyzers)
             {
                 // Dequeue reported analyzer diagnostics from the driver and store them in our maps.
-                var syntaxDiagnostics = driver.DequeueLocalDiagnostics(analyzer, syntax: true);
-                var semanticDiagnostics = driver.DequeueLocalDiagnostics(analyzer, syntax: false);
-                var compilationDiagnostics = driver.DequeueNonLocalDiagnostics(analyzer);
+                var syntaxDiagnostics = driver.DequeueLocalDiagnostics(analyzer, syntax: true, compilation: compilation);
+                var semanticDiagnostics = driver.DequeueLocalDiagnostics(analyzer, syntax: false, compilation: compilation);
+                var compilationDiagnostics = driver.DequeueNonLocalDiagnostics(analyzer, compilation);
 
                 lock (_gate)
                 {

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.CompilationData.cs

@@ -144,7 +144,7 @@ public void Free()
             }
         }
 
-        internal static CompilationData GetCachedCompilationData(Compilation compilation)
+        internal static CompilationData GetOrCreateCachedCompilationData(Compilation compilation)
         {
             return s_compilationDataCache.GetValue(compilation, c => new CompilationData(c));
         }
@@ -156,7 +156,7 @@ internal static bool RemoveCachedCompilationData(Compilation compilation)
 
         public static SemanticModel GetOrCreateCachedSemanticModel(SyntaxTree tree, Compilation compilation, CancellationToken cancellationToken)
         {
-            var compilationData = GetCachedCompilationData(compilation);
+            var compilationData = GetOrCreateCachedCompilationData(compilation);
             return compilationData.GetOrCreateCachedSemanticModel(tree, compilation, cancellationToken);
         }
 
@@ -169,13 +169,13 @@ public static bool RemoveCachedSemanticModel(SyntaxTree tree, Compilation compil
 
         public static bool TryGetCachedDeclaringReferences(ISymbol symbol, Compilation compilation, out ImmutableArray<SyntaxReference> declaringReferences)
         {
-            var compilationData = GetCachedCompilationData(compilation);
+            var compilationData = GetOrCreateCachedCompilationData(compilation);
             return compilationData.TryGetCachedDeclaringReferences(symbol, out declaringReferences);
         }
 
         public static void CacheDeclaringReferences(ISymbol symbol, Compilation compilation, ImmutableArray<SyntaxReference> declaringReferences)
         {
-            var compilationData = GetCachedCompilationData(compilation);
+            var compilationData = GetOrCreateCachedCompilationData(compilation);
             compilationData.CacheDeclaringReferences(symbol, declaringReferences);
         }
 

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs

@@ -140,9 +140,9 @@ private void Initialize(AnalyzerExecutor analyzerExecutor, DiagnosticQueue diagn
             Debug.Assert(_initializeTask == null);
 
             var diagnosticQueue = DiagnosticQueue.Create(categorizeDiagnostics);
-            var addDiagnostic = GetDiagnosticSinkWithSuppression(diagnosticQueue.Enqueue, compilation);
-            var addLocalDiagnosticOpt = categorizeDiagnostics ? GetDiagnosticSinkWithSuppression(diagnosticQueue.EnqueueLocal, compilation) : null;
-            var addNonLocalDiagnosticOpt = categorizeDiagnostics ? GetDiagnosticSinkWithSuppression(diagnosticQueue.EnqueueNonLocal, compilation) : null;
+            var addDiagnostic = GetDiagnosticSink(diagnosticQueue.Enqueue, compilation);
+            var addLocalDiagnosticOpt = categorizeDiagnostics ? GetDiagnosticSink(diagnosticQueue.EnqueueLocal, compilation) : null;
+            var addNonLocalDiagnosticOpt = categorizeDiagnostics ? GetDiagnosticSink(diagnosticQueue.EnqueueNonLocal, compilation) : null;
 
             Action<Exception, DiagnosticAnalyzer, Diagnostic> newOnAnalyzerException;
             if (analysisOptions.OnAnalyzerException != null)
@@ -374,7 +374,7 @@ private void ExecuteSyntaxTreeActions(AnalysisScope analysisScope, AnalysisState
         /// If <see cref="CompilationEventQueue"/> has been completed with all compilation events, then it waits for
         /// <see cref="WhenCompletedTask"/> task for the driver to finish processing all events and generate remaining analyzer diagnostics.
         /// </summary>
-        public async Task<ImmutableArray<Diagnostic>> GetDiagnosticsAsync()
+        public async Task<ImmutableArray<Diagnostic>> GetDiagnosticsAsync(Compilation compilation)
         {
             var allDiagnostics = DiagnosticBag.GetInstance();
             if (CompilationEventQueue.IsCompleted)
@@ -387,23 +387,61 @@ public async Task<ImmutableArray<Diagnostic>> GetDiagnosticsAsync()
                 }
             }
 
+            var suppressMessageState = GetOrCreateCachedCompilationData(compilation).SuppressMessageAttributeState;
             Diagnostic d;
             while (DiagnosticQueue.TryDequeue(out d))
             {
-                allDiagnostics.Add(d);
+                if (!suppressMessageState.IsDiagnosticSuppressed(d))
+                {
+                    allDiagnostics.Add(d);
+                }
             }
 
             return allDiagnostics.ToReadOnlyAndFree();
         }
 
-        public ImmutableArray<Diagnostic> DequeueLocalDiagnostics(DiagnosticAnalyzer analyzer, bool syntax)
+        public ImmutableArray<Diagnostic> DequeueLocalDiagnostics(DiagnosticAnalyzer analyzer, bool syntax, Compilation compilation)
         {
-            return syntax ? DiagnosticQueue.DequeueLocalSyntaxDiagnostics(analyzer) : DiagnosticQueue.DequeueLocalSemanticDiagnostics(analyzer);
+            var diagnostics = syntax ? DiagnosticQueue.DequeueLocalSyntaxDiagnostics(analyzer) : DiagnosticQueue.DequeueLocalSemanticDiagnostics(analyzer);
+            return FilterDiagnosticsSuppressedInSource(diagnostics, compilation);
         }
 
-        public ImmutableArray<Diagnostic> DequeueNonLocalDiagnostics(DiagnosticAnalyzer analyzer)
+        public ImmutableArray<Diagnostic> DequeueNonLocalDiagnostics(DiagnosticAnalyzer analyzer, Compilation compilation)
         {
-            return DiagnosticQueue.DequeueNonLocalDiagnostics(analyzer);
+            var diagnostics = DiagnosticQueue.DequeueNonLocalDiagnostics(analyzer);
+            return FilterDiagnosticsSuppressedInSource(diagnostics, compilation);
+        }
+
+        private static ImmutableArray<Diagnostic> FilterDiagnosticsSuppressedInSource(ImmutableArray<Diagnostic> diagnostics, Compilation compilation)
+        {
+            if (diagnostics.IsEmpty)
+            {
+                return diagnostics;
+            }
+
+            var suppressMessageState = GetOrCreateCachedCompilationData(compilation).SuppressMessageAttributeState;
+            ImmutableArray<Diagnostic>.Builder builder = null;
+            for (var i = 0; i < diagnostics.Length; i++)
+            {
+                var diagnostic = diagnostics[i];
+                if (suppressMessageState.IsDiagnosticSuppressed(diagnostic))
+                {
+                    if (builder == null)
+                    {
+                        builder = ImmutableArray.CreateBuilder<Diagnostic>();
+                        for (int j = 0; j < i; j++)
+                        {
+                            builder.Add(diagnostics[j]);
+                        }
+                    }
+                }
+                else if (builder != null)
+                {
+                    builder.Add(diagnostic);
+                }
+            }
+
+            return builder != null ? builder.ToImmutable() : diagnostics; 
         }
 
         /// <summary>
@@ -668,18 +706,13 @@ private void ExecuteSymbolActions(SymbolDeclaredCompilationEvent symbolEvent, An
                 return;
             }
 
-            Action<Diagnostic> addDiagnosticForSymbol = GetDiagnosticSinkWithSuppression(DiagnosticQueue.Enqueue, symbolEvent.Compilation, symbol);
-            Action<Diagnostic, DiagnosticAnalyzer, bool> addLocalDiagnosticForSymbol = analysisScope.CategorizeDiagnostics ? GetDiagnosticSinkWithSuppression(DiagnosticQueue.EnqueueLocal, symbolEvent.Compilation, symbol) : null;
-            Action<Diagnostic, DiagnosticAnalyzer> addNonLocalDiagnosticForSymbol = analysisScope.CategorizeDiagnostics ? GetDiagnosticSinkWithSuppression(DiagnosticQueue.EnqueueNonLocal, symbolEvent.Compilation, symbol) : null;
-
             foreach (var analyzer in analysisScope.Analyzers)
             {
                 // Invoke symbol analyzers only for source symbols.
                 ImmutableArray<ImmutableArray<SymbolAnalyzerAction>> actionsByKind;
                 if (_symbolActionsByKind.TryGetValue(analyzer, out actionsByKind) && (int)symbol.Kind < actionsByKind.Length)
                 {
-                    analyzerExecutor.ExecuteSymbolActions(actionsByKind[(int)symbol.Kind], analyzer, symbol, addDiagnosticForSymbol,
-                        addLocalDiagnosticForSymbol, addNonLocalDiagnosticForSymbol, GetTopmostNodeForAnalysis, analysisScope, analysisStateOpt);
+                    analyzerExecutor.ExecuteSymbolActions(actionsByKind[(int)symbol.Kind], analyzer, symbol, GetTopmostNodeForAnalysis, analysisScope, analysisStateOpt);
                 }
                 else
                 {
@@ -773,11 +806,11 @@ private void ProcessCompilationCompleted(CompilationCompletedEvent endEvent, Ana
             }
         }
 
-        internal static Action<Diagnostic> GetDiagnosticSinkWithSuppression(Action<Diagnostic> addDiagnosticCore, Compilation compilation, ISymbol symbolOpt = null)
+        internal static Action<Diagnostic> GetDiagnosticSink(Action<Diagnostic> addDiagnosticCore, Compilation compilation)
         {
             return diagnostic =>
             {
-                var filteredDiagnostic = GetFilteredDiagnostic(diagnostic, compilation, symbolOpt);
+                var filteredDiagnostic = GetFilteredDiagnostic(diagnostic, compilation);
                 if (filteredDiagnostic != null)
                 {
                     addDiagnosticCore(filteredDiagnostic);
@@ -785,11 +818,11 @@ internal static Action<Diagnostic> GetDiagnosticSinkWithSuppression(Action<Diagn
             };
         }
 
-        internal static Action<Diagnostic, DiagnosticAnalyzer, bool> GetDiagnosticSinkWithSuppression(Action<Diagnostic, DiagnosticAnalyzer, bool> addLocalDiagnosticCore, Compilation compilation, ISymbol symbolOpt = null)
+        internal static Action<Diagnostic, DiagnosticAnalyzer, bool> GetDiagnosticSink(Action<Diagnostic, DiagnosticAnalyzer, bool> addLocalDiagnosticCore, Compilation compilation)
         {
             return (diagnostic, analyzer, isSyntaxDiagnostic) =>
             {
-                var filteredDiagnostic = GetFilteredDiagnostic(diagnostic, compilation, symbolOpt);
+                var filteredDiagnostic = GetFilteredDiagnostic(diagnostic, compilation);
                 if (filteredDiagnostic != null)
                 {
                     addLocalDiagnosticCore(filteredDiagnostic, analyzer, isSyntaxDiagnostic);
@@ -797,11 +830,11 @@ internal static Action<Diagnostic> GetDiagnosticSinkWithSuppression(Action<Diagn
             };
         }
 
-        internal static Action<Diagnostic, DiagnosticAnalyzer> GetDiagnosticSinkWithSuppression(Action<Diagnostic, DiagnosticAnalyzer> addNonLocalDiagnosticCore, Compilation compilation, ISymbol symbolOpt = null)
+        internal static Action<Diagnostic, DiagnosticAnalyzer> GetDiagnosticSink(Action<Diagnostic, DiagnosticAnalyzer> addNonLocalDiagnosticCore, Compilation compilation)
         {
             return (diagnostic, analyzer) =>
             {
-                var filteredDiagnostic = GetFilteredDiagnostic(diagnostic, compilation, symbolOpt);
+                var filteredDiagnostic = GetFilteredDiagnostic(diagnostic, compilation);
                 if (filteredDiagnostic != null)
                 {
                     addNonLocalDiagnosticCore(filteredDiagnostic, analyzer);
@@ -809,19 +842,9 @@ internal static Action<Diagnostic> GetDiagnosticSinkWithSuppression(Action<Diagn
             };
         }
 
-        private static Diagnostic GetFilteredDiagnostic(Diagnostic diagnostic, Compilation compilation, ISymbol symbolOpt = null)
+        private static Diagnostic GetFilteredDiagnostic(Diagnostic diagnostic, Compilation compilation)
         {
-            var filteredDiagnostic = compilation.Options.FilterDiagnostic(diagnostic);
-            if (filteredDiagnostic != null)
-            {
-                var suppressMessageState = GetCachedCompilationData(compilation).SuppressMessageAttributeState;
-                if (suppressMessageState.IsDiagnosticSuppressed(filteredDiagnostic, symbolOpt: symbolOpt))
-                {
-                    return null;
-                }
-            }
-
-            return filteredDiagnostic;
+            return compilation.Options.FilterDiagnostic(diagnostic);
         }
 
         private static Task<AnalyzerActions> GetAnalyzerActionsAsync(
@@ -1096,7 +1119,7 @@ private bool ShouldExecuteCodeBlockActions(AnalysisScope analysisScope, ISymbol
             // NOTE: The driver guarantees that only a single thread will be performing analysis on individual declaration.
             // However, there might be multiple threads analyzing different trees at the same time, so we need to lock the map for read/write.
 
-            var map = GetCachedCompilationData(compilation).DeclarationAnalysisDataMap;
+            var map = GetOrCreateCachedCompilationData(compilation).DeclarationAnalysisDataMap;
 
             DeclarationAnalysisData data;
             lock (map)

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerExecutor.cs

@@ -245,9 +245,6 @@ private void ExecuteCompilationActionsCore(ImmutableArray<CompilationAnalyzerAct
         /// <param name="symbolActions">Symbol actions to be executed.</param>
         /// <param name="analyzer">Analyzer whose actions are to be executed.</param>
         /// <param name="symbol">Symbol to be analyzed.</param>
-        /// <param name="overriddenAddDiagnostic">Overridden add diagnostic delegate.</param>
-        /// <param name="overriddenAddLocalDiagnostic">Overridden add local diagnostic delegate.</param>
-        /// <param name="overriddenAddNonLocalDiagnostic">Overridden add non-local diagnostic delegate.</param>
         /// <param name="getTopMostNodeForAnalysis">Delegate to get topmost declaration node for a symbol declaration reference.</param>
         /// <param name="analysisScope">Scope for analyzer execution.</param>
         /// <param name="analysisStateOpt">An optional object to track analysis state.</param>
@@ -255,9 +252,6 @@ private void ExecuteCompilationActionsCore(ImmutableArray<CompilationAnalyzerAct
             ImmutableArray<SymbolAnalyzerAction> symbolActions,
             DiagnosticAnalyzer analyzer,
             ISymbol symbol,
-            Action<Diagnostic> overriddenAddDiagnostic,
-            Action<Diagnostic, DiagnosticAnalyzer, bool> overriddenAddLocalDiagnostic,
-            Action<Diagnostic, DiagnosticAnalyzer> overriddenAddNonLocalDiagnostic,
             Func<ISymbol, SyntaxReference, Compilation, SyntaxNode> getTopMostNodeForAnalysis,
             AnalysisScope analysisScope,
             AnalysisState analysisStateOpt)
@@ -268,8 +262,7 @@ private void ExecuteCompilationActionsCore(ImmutableArray<CompilationAnalyzerAct
             {
                 if (TryStartAnalyzingSymbol(symbol, analyzer, analysisScope, analysisStateOpt, out analyzerStateOpt))
                 {
-                    ExecuteSymbolActionsCore(symbolActions, analyzer, symbol, overriddenAddDiagnostic,
-                        overriddenAddLocalDiagnostic, overriddenAddNonLocalDiagnostic, getTopMostNodeForAnalysis, analyzerStateOpt);
+                    ExecuteSymbolActionsCore(symbolActions, analyzer, symbol, getTopMostNodeForAnalysis, analyzerStateOpt);
                     analysisStateOpt?.MarkSymbolComplete(symbol, analyzer);
                 }
             }
@@ -283,16 +276,12 @@ private void ExecuteCompilationActionsCore(ImmutableArray<CompilationAnalyzerAct
             ImmutableArray<SymbolAnalyzerAction> symbolActions,
             DiagnosticAnalyzer analyzer,
             ISymbol symbol,
-            Action<Diagnostic> overriddenAddDiagnostic,
-            Action<Diagnostic, DiagnosticAnalyzer, bool> overriddenAddLocalDiagnostic,
-            Action<Diagnostic, DiagnosticAnalyzer> overriddenAddNonLocalDiagnostic,
             Func<ISymbol, SyntaxReference, Compilation, SyntaxNode> getTopMostNodeForAnalysis,
             AnalyzerStateData analyzerStateOpt)
         {
-            Debug.Assert(overriddenAddLocalDiagnostic == null || overriddenAddDiagnostic != null);
             Debug.Assert(getTopMostNodeForAnalysis != null);
 
-            var addDiagnostic = GetAddDiagnostic(symbol, _compilation, analyzer, overriddenAddDiagnostic ?? _addDiagnostic, overriddenAddLocalDiagnostic ?? _addLocalDiagnosticOpt, overriddenAddNonLocalDiagnostic ?? _addNonLocalDiagnosticOpt, getTopMostNodeForAnalysis);
+            var addDiagnostic = GetAddDiagnostic(symbol, _compilation, analyzer, _addDiagnostic, _addLocalDiagnosticOpt, _addNonLocalDiagnosticOpt, getTopMostNodeForAnalysis);
 
             foreach (var symbolAction in symbolActions)
             {

src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationWithAnalyzers.cs

@@ -688,7 +688,7 @@ private async Task ComputeAnalyzerDiagnosticsCoreAsync(AnalyzerDriver driver, As
                     finally
                     {
                         // Update the diagnostic results based on the diagnostics reported on the driver.
-                        _analysisResult.StoreAnalysisResult(analysisScope, driver);
+                        _analysisResult.StoreAnalysisResult(analysisScope, driver, _compilation);
                     }
                 }
             }
@@ -963,7 +963,7 @@ public static IEnumerable<Diagnostic> GetEffectiveDiagnostics(IEnumerable<Diagno
                 throw new ArgumentNullException(nameof(compilation));
             }
 
-            var suppressMessageState = AnalyzerDriver.GetCachedCompilationData(compilation).SuppressMessageAttributeState;
+            var suppressMessageState = AnalyzerDriver.GetOrCreateCachedCompilationData(compilation).SuppressMessageAttributeState;
             foreach (var diagnostic in diagnostics.ToImmutableArray())
             {
                 if (diagnostic != null)

src/Test/Utilities/Shared/Diagnostics/DiagnosticExtensions.cs

@@ -170,7 +170,7 @@ public static TCompilation VerifyDiagnostics<TCompilation>(this TCompilation c,
             Compilation newCompilation;
             var driver = AnalyzerDriver.CreateAndAttachToCompilation(c, analyzersArray, options, AnalyzerManager.Instance, onAnalyzerException, false, out newCompilation, CancellationToken.None);
             var discarded = newCompilation.GetDiagnostics();
-            diagnostics = driver.GetDiagnosticsAsync().Result.AddRange(exceptionDiagnostics);
+            diagnostics = driver.GetDiagnosticsAsync(newCompilation).Result.AddRange(exceptionDiagnostics);
 
             return (TCompilation)newCompilation; // note this is a new compilation
         }