Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix race condition in computing _hostAnalyzerStateMap #57648

Merged
merged 8 commits into from
Nov 11, 2021
Merged

Fix race condition in computing _hostAnalyzerStateMap #57648

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1e66c27
Fix race condition in computing _hostAnalyzerStateMap
mavasani Nov 8, 2021
03074fa
Address feedback
mavasani Nov 10, 2021
e60b5fd
Update src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncr…
mavasani Nov 10, 2021
fd2d637
Fix build
mavasani Nov 10, 2021
ded01a1
Implement IEquatable<HostAnalyzerStateSetKey>
mavasani Nov 10, 2021
9c84713
Address feedback
mavasani Nov 11, 2021
d242cd8
Ensure that GetAllHostStateSets do not return duplicates from prior s…
mavasani Nov 11, 2021
255f924
Update src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncr…
mavasani Nov 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions ...re/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ public IEnumerable<StateSet> GetAllHostStateSets()

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

static HostAnalyzerStateSets CreateLanguageSpecificAnalyzerMap(string language, HostDiagnosticAnalyzers hostAnalyzers)
static HostAnalyzerStateSets CreateLanguageSpecificAnalyzerMap(HostAnalyzerStateSetKey arg)
{
var language = arg.Language;
var hostAnalyzers = arg.Analyzers;
var analyzersPerReference = hostAnalyzers.GetOrCreateHostDiagnosticAnalyzersPerReference(language);

var analyzerMap = CreateStateSetMap(language, analyzersPerReference.Values, includeFileContentLoadAnalyzer: true);
Expand Down
26 changes: 24 additions & 2 deletions ...Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host;
Expand All @@ -27,7 +28,7 @@ private partial class StateManager
/// Analyzers supplied by the host (IDE). These are built-in to the IDE, the compiler, or from an installed IDE extension (VSIX).
/// Maps language name to the analyzers and their state.
/// </summary>
private ImmutableDictionary<string, HostAnalyzerStateSets> _hostAnalyzerStateMap;
private ImmutableDictionary<HostAnalyzerStateSetKey, HostAnalyzerStateSets> _hostAnalyzerStateMap;

/// <summary>
/// Analyzers referenced by the project via a PackageReference.
Expand All @@ -43,7 +44,7 @@ public StateManager(DiagnosticAnalyzerInfoCache analyzerInfoCache)
{
_analyzerInfoCache = analyzerInfoCache;

_hostAnalyzerStateMap = ImmutableDictionary<string, HostAnalyzerStateSets>.Empty;
_hostAnalyzerStateMap = ImmutableDictionary<HostAnalyzerStateSetKey, HostAnalyzerStateSets>.Empty;
_projectAnalyzerStateMap = new ConcurrentDictionary<ProjectId, ProjectAnalyzerStateSets>(concurrencyLevel: 2, capacity: 10);
}

Expand Down Expand Up @@ -308,6 +309,27 @@ private void VerifyProjectDiagnosticStates(IEnumerable<StateSet> stateSets)

VerifyUniqueStateNames(hostStates.Concat(stateSets));
}

private readonly struct HostAnalyzerStateSetKey : IEquatable<HostAnalyzerStateSetKey>
{
public HostAnalyzerStateSetKey(string language, HostDiagnosticAnalyzers analyzers)
{
Language = language;
Analyzers = analyzers;
}

public string Language { get; }
public HostDiagnosticAnalyzers Analyzers { get; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 I requested that instead of using HostDiagnosticAnalyzers as the key directly, we use HostDiagnosticAnalyzers.GetHostAnalyzerReferencesMap(). This has the same net result for the keys but ensures we don't accidentally GC root the DiagnosticAnalyzer instances themselves (from other fields in HostDiagnosticAnalyzers) in the off chance this cache ends up with unforeseen items.


public bool Equals(HostAnalyzerStateSetKey other)
=> Language == other.Language && Analyzers == other.Analyzers;

public override bool Equals(object? obj)
=> obj is HostAnalyzerStateSetKey key && Equals(key);

public override int GetHashCode()
=> Hash.Combine(Language.GetHashCode(), Analyzers.GetHashCode());
}
}
}
}