Switch HashSet to ConcurrentDictionary for VS Extension Fix Cache (#581)

microsoft · Aug 23, 2023 · 9713e84 · 9713e84
1 parent fba56c2
commit 9713e84
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 13 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.0.19] - 2023-08-22
+### VS Extension
+Fix concurrent access issue with cache storage for fixes. Fix #480
+
 ## [1.0.18] - 2023-08-09
 ### Rules
 Fix language filtering on random number generator rules. Fix #468

diff --git a/DevSkim-DotNet/Microsoft.DevSkim.LanguageProtoInterop/CodeFixMapping.cs b/DevSkim-DotNet/Microsoft.DevSkim.LanguageProtoInterop/CodeFixMapping.cs
@@ -16,7 +16,7 @@ public class MappingsVersion
         public Uri fileName;
     }
 
-    public class CodeFixMapping
+    public class CodeFixMapping : IEquatable<CodeFixMapping>
     {
         /// <summary>
         /// Reported version of the document the diagnostic applies to
@@ -73,5 +73,36 @@ public CodeFixMapping(Diagnostic diagnostic, string replacement, Uri fileName, s
             this.matchEnd = matchEnd;
             this.isSuppression = isSuppression;
         }
+
+        public bool Equals(CodeFixMapping other)
+        {
+            if (ReferenceEquals(null, other)) return false;
+            if (ReferenceEquals(this, other)) return true;
+            return version == other.version && Equals(diagnostic, other.diagnostic) && replacement == other.replacement && Equals(fileName, other.fileName) && friendlyString == other.friendlyString && matchStart == other.matchStart && matchEnd == other.matchEnd && isSuppression == other.isSuppression;
+        }
+
+        public override bool Equals(object obj)
+        {
+            if (ReferenceEquals(null, obj)) return false;
+            if (ReferenceEquals(this, obj)) return true;
+            if (obj.GetType() != this.GetType()) return false;
+            return Equals((CodeFixMapping)obj);
+        }
+
+        public override int GetHashCode()
+        {
+            unchecked
+            {
+                var hashCode = version.GetHashCode();
+                hashCode = (hashCode * 397) ^ (diagnostic != null ? diagnostic.GetHashCode() : 0);
+                hashCode = (hashCode * 397) ^ (replacement != null ? replacement.GetHashCode() : 0);
+                hashCode = (hashCode * 397) ^ (fileName != null ? fileName.GetHashCode() : 0);
+                hashCode = (hashCode * 397) ^ (friendlyString != null ? friendlyString.GetHashCode() : 0);
+                hashCode = (hashCode * 397) ^ matchStart;
+                hashCode = (hashCode * 397) ^ matchEnd;
+                hashCode = (hashCode * 397) ^ isSuppression.GetHashCode();
+                return hashCode;
+            }
+        }
     }
 }
diff --git a/DevSkim-DotNet/Microsoft.DevSkim.VisualStudio/DevSkimFixMessageTarget.cs b/DevSkim-DotNet/Microsoft.DevSkim.VisualStudio/DevSkimFixMessageTarget.cs
@@ -58,15 +58,22 @@ await Task.Run(() =>
                 {
                     StaticData.FileToCodeFixMap.AddOrUpdate(mapping.fileName,
                     // Add New Nested Dictionary
-                    (Uri _) => new ConcurrentDictionary<int, HashSet<CodeFixMapping>>(new Dictionary<int, HashSet<CodeFixMapping>>() { { mapping.version ?? -1, new HashSet<CodeFixMapping>() { mapping } } }),
+                    (Uri _) => new (new Dictionary<int, ConcurrentDictionary<CodeFixMapping, bool>> 
+                    { { mapping.version ?? -1, new (new Dictionary<CodeFixMapping, bool>() 
+                        { {mapping, true } }) } }),
                     // Update Nested Dictionary
                     (key, oldValue) =>
                     {
                         oldValue.AddOrUpdate(mapping.version ?? -1,
-                            // Add new HashSet
-                            (int _) => new HashSet<CodeFixMapping>() { mapping },
-                            // Update HashSet of CodeFixMappings
-                            (versionKey, oldSet) => { oldSet.Add(mapping); return oldSet; });
+                            // Add new Set of mappings
+                            (int _) =>
+                            {
+                                var addedMapping = new ConcurrentDictionary<CodeFixMapping, bool>();
+                                addedMapping.TryAdd(mapping, true);
+                                return addedMapping;
+                            },
+                            // Update Set of CodeFixMappings
+                            (versionKey, oldSet) => { oldSet.TryAdd(mapping, true); return oldSet; });
                         return oldValue;
                     });
                 }

diff --git a/DevSkim-DotNet/Microsoft.DevSkim.VisualStudio/StaticData.cs b/DevSkim-DotNet/Microsoft.DevSkim.VisualStudio/StaticData.cs
@@ -8,6 +8,6 @@
     internal static class StaticData
     {
         // Maps file name to a dictionary of file versions to a deduplicated set of CodeFixMappings
-        internal static ConcurrentDictionary<Uri, ConcurrentDictionary<int, HashSet<CodeFixMapping>>> FileToCodeFixMap { get; } = new ConcurrentDictionary<Uri, ConcurrentDictionary<int, HashSet<CodeFixMapping>>>();
+        internal static ConcurrentDictionary<Uri, ConcurrentDictionary<int, ConcurrentDictionary<CodeFixMapping, bool>>> FileToCodeFixMap { get; } = new();
     }
 }
diff --git a/DevSkim-DotNet/Microsoft.DevSkim.VisualStudio/SuggestionActionsSource.cs b/DevSkim-DotNet/Microsoft.DevSkim.VisualStudio/SuggestionActionsSource.cs
@@ -6,6 +6,7 @@
 using Microsoft.VisualStudio.Text.Editor;
 using Microsoft.VisualStudio.Text.Operations;
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
@@ -43,11 +44,11 @@ public IEnumerable<SuggestedActionSet> GetSuggestedActions(ISuggestedActionCateg
             List<ISuggestedAction> suggestedActions = new List<ISuggestedAction>();
             if (TryGetWordUnderCaret(out TextExtent wordExtent) && wordExtent.IsSignificant)
             {
-                if (StaticData.FileToCodeFixMap.TryGetValue(new Uri(_fileName), out System.Collections.Concurrent.ConcurrentDictionary<int, HashSet<CodeFixMapping>> dictForFile))
+                if (StaticData.FileToCodeFixMap.TryGetValue(new Uri(_fileName), out ConcurrentDictionary<int, ConcurrentDictionary<CodeFixMapping, bool>> dictForFile))
                 {
-                    if (dictForFile.TryGetValue(wordExtent.Span.Snapshot.Version.VersionNumber, out HashSet<CodeFixMapping> fixes))
+                    if (dictForFile.TryGetValue(wordExtent.Span.Snapshot.Version.VersionNumber, out ConcurrentDictionary<CodeFixMapping, bool> fixes))
                     {
-                        suggestedActions.AddRange(fixes.Where(codeFixMapping => Intersects(codeFixMapping, wordExtent)).Select(intersectedMapping => new DevSkimSuggestedAction(wordExtent.Span, intersectedMapping)));
+                        suggestedActions.AddRange(fixes.Where(codeFixMapping => Intersects(codeFixMapping.Key, wordExtent)).Select(intersectedMapping => new DevSkimSuggestedAction(wordExtent.Span, intersectedMapping.Key)));
                     }
                 }
                 yield return new SuggestedActionSet(suggestedActions, wordExtent.Span);
@@ -79,11 +80,11 @@ public Task<bool> HasSuggestedActionsAsync(ISuggestedActionCategorySet requested
                 bool res = TryGetWordUnderCaret(out TextExtent wordExtent);
                 if (res && wordExtent.IsSignificant)
                 {
-                    if (StaticData.FileToCodeFixMap.TryGetValue(new Uri(_fileName), out System.Collections.Concurrent.ConcurrentDictionary<int, HashSet<CodeFixMapping>> dictForFile))
+                    if (StaticData.FileToCodeFixMap.TryGetValue(new Uri(_fileName), out ConcurrentDictionary<int, ConcurrentDictionary<CodeFixMapping, bool>> dictForFile))
                     {
-                        if (dictForFile.TryGetValue(wordExtent.Span.Snapshot.Version.VersionNumber, out HashSet<CodeFixMapping> fixes))
+                        if (dictForFile.TryGetValue(wordExtent.Span.Snapshot.Version.VersionNumber, out ConcurrentDictionary<CodeFixMapping, bool> fixes))
                         {
-                            return fixes.Any(codeFixMapping => Intersects(codeFixMapping, wordExtent));
+                            return fixes.Any(codeFixMapping => Intersects(codeFixMapping.Key, wordExtent));
                         }
                     }
                 }