Skip to content

Commit

Permalink
Fix leak in _hostAnalyzerStateMap
Browse files Browse the repository at this point in the history
Fixes [AB#1449967](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1449967)

dotnet#57648 fixed a race condition in computing the host analyzer references, but introduced a leak in _hostAnalyzerStateMap, wherein each new solution opened ends up creating a new entry in this map, even though the underlying set of host analyzer references doesn't change. This is due to the fact that each new SolutionState instance creates a new instance of HostDiagnosticAnalyzers, whose constructor creates a new map from analyzer references ID to the analyzer reference. This map is used as part of the key for _hostAnalyzerStateMap, leading to the leak.

This change fixes the leak by using the underlying list of host analyzer references as part of the key for _hostAnalyzerStateMap, which is reference equals across all solution instances opened in the VS session. Verified manually that after this fix, repeatedly opening and closing a solution in VS no longer leads to unbounded growth of _hostAnalyzerStateMap.
  • Loading branch information
mavasani committed Dec 18, 2021
1 parent 834bc3c commit 8df82f3
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests
Assert.Equal(workspaceDiagnosticAnalyzer.DiagDescriptor.Id, descriptors(0).Id)

' Add an existing workspace analyzer to the project, ensure no duplicate diagnostics.
Dim duplicateProjectAnalyzersReference = hostAnalyzers.GetHostAnalyzerReferencesMap().First().Value
project = project.WithAnalyzerReferences({duplicateProjectAnalyzersReference})
project = project.WithAnalyzerReferences(hostAnalyzers.HostAnalyzerReferences)

' Verify duplicate descriptors or diagnostics.
descriptorsMap = hostAnalyzers.GetDiagnosticDescriptorsPerReference(diagnosticService.AnalyzerInfoCache, project)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ private partial class StateManager
{
public IEnumerable<StateSet> GetAllHostStateSets()
{
var analyzerReferencesMap = _workspace.CurrentSolution.State.Analyzers.GetHostAnalyzerReferencesMap();
var analyzerReferences = _workspace.CurrentSolution.State.Analyzers.HostAnalyzerReferences;
foreach (var (key, value) in _hostAnalyzerStateMap)
{
if (key.AnalyzerReferences == analyzerReferencesMap)
if (key.AnalyzerReferences == analyzerReferences)
{
foreach (var stateSet in value.OrderedStateSets)
{
Expand All @@ -31,7 +31,7 @@ public IEnumerable<StateSet> GetAllHostStateSets()

private HostAnalyzerStateSets GetOrCreateHostStateSets(Project project, ProjectAnalyzerStateSets projectStateSets)
{
var key = new HostAnalyzerStateSetKey(project.Language, project.Solution.State.Analyzers.GetHostAnalyzerReferencesMap());
var key = new HostAnalyzerStateSetKey(project.Language, project.Solution.State.Analyzers.HostAnalyzerReferences);
var hostStateSets = ImmutableInterlocked.GetOrAdd(ref _hostAnalyzerStateMap, key, CreateLanguageSpecificAnalyzerMap, project.Solution.State.Analyzers);
return hostStateSets.WithExcludedAnalyzers(projectStateSets.SkippedAnalyzersInfo.SkippedAnalyzers);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,14 +314,14 @@ private void VerifyProjectDiagnosticStates(IEnumerable<StateSet> stateSets)

private readonly struct HostAnalyzerStateSetKey : IEquatable<HostAnalyzerStateSetKey>
{
public HostAnalyzerStateSetKey(string language, ImmutableDictionary<object, AnalyzerReference> analyzerReferences)
public HostAnalyzerStateSetKey(string language, IReadOnlyList<AnalyzerReference> analyzerReferences)
{
Language = language;
AnalyzerReferences = analyzerReferences;
}

public string Language { get; }
public ImmutableDictionary<object, AnalyzerReference> AnalyzerReferences { get; }
public IReadOnlyList<AnalyzerReference> AnalyzerReferences { get; }

public bool Equals(HostAnalyzerStateSetKey other)
=> Language == other.Language && AnalyzerReferences == other.AnalyzerReferences;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ internal sealed class HostDiagnosticAnalyzers
/// </remarks>
private readonly ConditionalWeakTable<IReadOnlyList<AnalyzerReference>, StrongBox<ImmutableDictionary<string, SkippedHostAnalyzersInfo>>> _skippedHostAnalyzers;

internal HostDiagnosticAnalyzers(IEnumerable<AnalyzerReference> hostAnalyzerReferences)
internal HostDiagnosticAnalyzers(IReadOnlyList<AnalyzerReference> hostAnalyzerReferences)
{
HostAnalyzerReferences = hostAnalyzerReferences;
_hostAnalyzerReferencesMap = CreateAnalyzerReferencesMap(hostAnalyzerReferences);
_hostDiagnosticAnalyzersPerLanguageMap = new ConcurrentDictionary<string, ImmutableDictionary<object, ImmutableArray<DiagnosticAnalyzer>>>(concurrencyLevel: 2, capacity: 2);
_lazyHostDiagnosticAnalyzersPerReferenceMap = new Lazy<ImmutableDictionary<object, ImmutableArray<DiagnosticAnalyzer>>>(() => CreateDiagnosticAnalyzersPerReferenceMap(_hostAnalyzerReferencesMap), isThreadSafe: true);
Expand All @@ -64,10 +65,9 @@ internal HostDiagnosticAnalyzers(IEnumerable<AnalyzerReference> hostAnalyzerRefe
}

/// <summary>
/// It returns a map with <see cref="AnalyzerReference.Id"/> as key and <see cref="AnalyzerReference"/> as value
/// List of host <see cref="AnalyzerReference"/>s
/// </summary>
public ImmutableDictionary<object, AnalyzerReference> GetHostAnalyzerReferencesMap()
=> _hostAnalyzerReferencesMap;
public IReadOnlyList<AnalyzerReference> HostAnalyzerReferences { get; }

/// <summary>
/// Get <see cref="AnalyzerReference"/> identity and <see cref="DiagnosticAnalyzer"/>s map for given <paramref name="language"/>
Expand Down

0 comments on commit 8df82f3

Please sign in to comment.