Move off of a linear walk in solution-explorer symbol tree updates.

src/VisualStudio/Core/Impl/SolutionExplorer/SymbolTree/RootSymbolTreeItemCollectionSource.cs

@@ -4,7 +4,6 @@
 
 using System;
 using System.Collections;
-using System.Collections.Generic;
 using System.ComponentModel;
 using System.IO;
 using System.Threading;
@@ -48,29 +47,26 @@ private sealed class RootSymbolTreeItemCollectionSource(
         public void Reset()
         {
             _rootProvider.ThreadingContext.ThrowIfNotOnUIThread();
-            _hasEverBeenExpanded = 0;
             _childCollection.ResetToUncomputedState(GetHasItemsDefaultValue(_hierarchyItem));
+
+            // Note: we intentionally do not touch _hasEverBeenExpanded.  The platform only ever calls "Items"
+            // at most once (even if we notify that it changed). So if we reset _hasEverBeenExpanded to 0, then
+            // it will never leave that state from that point on, and we'll be stuck in an invalid state.
         }
 
-        public async Task UpdateIfAffectedAsync(
-            HashSet<string> updatedFilePaths,
-            CancellationToken cancellationToken)
+        public async Task UpdateIfEverExpandedAsync(CancellationToken cancellationToken)
         {
             // If we haven't been initialized yet, then we don't have to do anything.  We will get called again
             // in the future as documents are mutated, and we'll ignore until the point that the user has at
             // least expanded this node once.
             if (_hasEverBeenExpanded == 0)
                 return;
 
-            var filePath = TryGetCanonicalName();
-            if (filePath != null && !updatedFilePaths.Contains(filePath))
-                return;
-
             // Try to find a roslyn document for this file path.  Note: it is intentional that we continue onwards,
             // even if this returns null.  We still want to put ourselves into the final "i have no items" state,
             // instead of bailing out and potentially leaving either stale items, or leaving ourselves in the 
             // "i don't know what items are in me" state.
-            var documentId = DetermineDocumentId(filePath);
+            var documentId = DetermineDocumentId();
 
             var solution = _rootProvider._workspace.CurrentSolution;
 
@@ -95,27 +91,10 @@ public async Task UpdateIfAffectedAsync(
             }
         }
 
-        private string? TryGetCanonicalName()
+        private DocumentId? DetermineDocumentId()
         {
-            // Quick check that will be correct the majority of the time.
-            if (!_hierarchyItem.IsDisposed)
-            {
-                // We are running in the background.  So it's possible that the type may be disposed between
-                // the above check and retrieving the canonical name.  So have to guard against that just in case.
-                try
-                {
-                    return _hierarchyItem.CanonicalName;
-                }
-                catch (ObjectDisposedException)
-                {
-                }
-            }
-
-            return null;
-        }
+            var filePath = TryGetCanonicalName();
 
-        private DocumentId? DetermineDocumentId(string? filePath)
-        {
             if (filePath != null)
             {
                 var idMap = _rootProvider._workspace.Services.GetRequiredService<IHierarchyItemToProjectIdMap>();
@@ -127,6 +106,25 @@ public async Task UpdateIfAffectedAsync(
             }
 
             return null;
+
+            string? TryGetCanonicalName()
+            {
+                // Quick check that will be correct the majority of the time.
+                if (!_hierarchyItem.IsDisposed)
+                {
+                    // We are running in the background.  So it's possible that the type may be disposed between
+                    // the above check and retrieving the canonical name.  So have to guard against that just in case.
+                    try
+                    {
+                        return _hierarchyItem.CanonicalName;
+                    }
+                    catch (ObjectDisposedException)
+                    {
+                    }
+                }
+
+                return null;
+            }
         }
 
         object IAttachedCollectionSource.SourceItem => _childCollection.SourceItem;

src/VisualStudio/Core/Impl/SolutionExplorer/SymbolTree/RootSymbolTreeItemSourceProvider.cs

@@ -3,13 +3,11 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Collections.Concurrent;
-using System.Collections.Generic;
-using System.Collections.Immutable;
 using System.ComponentModel;
 using System.ComponentModel.Composition;
 using System.Threading;
 using System.Threading.Tasks;
+using ICSharpCode.Decompiler.Util;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
@@ -40,7 +38,14 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.SolutionExplore
 [AppliesToProject("CSharp | VB")]
 internal sealed partial class RootSymbolTreeItemSourceProvider : AttachedCollectionSourceProvider<IVsHierarchyItem>
 {
-    private readonly ConcurrentDictionary<IVsHierarchyItem, RootSymbolTreeItemCollectionSource> _hierarchyToCollectionSource = [];
+    /// <summary>
+    /// Mapping from filepath to the collection sources made for it.  Is a multi dictionary because the same
+    /// file may appear in multiple projects, but each will have its own collection soure to represent the view
+    /// of that file through that project.
+    /// </summary>
+    /// <remarks>Lock this instance when reading/writing as it is used over different threads.</remarks>
+    private readonly MultiDictionary<string, RootSymbolTreeItemCollectionSource> _filePathToCollectionSources = new(
+        StringComparer.OrdinalIgnoreCase);
 
     /// <summary>
     /// Queue of notifications we've heard about for changed document file paths.  We'll then go update the
@@ -106,19 +111,21 @@ public RootSymbolTreeItemSourceProvider(
     private async ValueTask UpdateCollectionSourcesAsync(
         ImmutableSegmentedList<string> updatedFilePaths, CancellationToken cancellationToken)
     {
-        using var _1 = SharedPools.StringIgnoreCaseHashSet.GetPooledObject(out var filePathSet);
-        using var _2 = Microsoft.CodeAnalysis.PooledObjects.ArrayBuilder<RootSymbolTreeItemCollectionSource>.GetInstance(out var sources);
+        using var _ = Microsoft.CodeAnalysis.PooledObjects.ArrayBuilder<RootSymbolTreeItemCollectionSource>.GetInstance(out var sources);
 
-        filePathSet.AddRange(updatedFilePaths);
-        sources.AddRange(_hierarchyToCollectionSource.Values);
+        lock (_filePathToCollectionSources)
+        {
+            foreach (var filePath in updatedFilePaths)
+                sources.AddRange(_filePathToCollectionSources[filePath]);
+        }
 
         // Update all the affected documents in parallel.
         await RoslynParallel.ForEachAsync(
             sources,
             cancellationToken,
             async (source, cancellationToken) =>
             {
-                await source.UpdateIfAffectedAsync(filePathSet, cancellationToken)
+                await source.UpdateIfEverExpandedAsync(cancellationToken)
                     .ReportNonFatalErrorUnlessCancelledAsync(cancellationToken)
                     .ConfigureAwait(false);
             }).ConfigureAwait(false);
@@ -150,11 +157,16 @@ await source.UpdateIfAffectedAsync(filePathSet, cancellationToken)
             return null;
         }
 
-        if (item.CanonicalName is not string filePath)
+        // Important: currentFilePath is mutable state captured *AND UPDATED* in the local function  
+        // OnItemPropertyChanged below.  It allows us to know the file path of the item *prior* to
+        // it being changed *when* we hear the update about it having changed (since hte event doesn't
+        // contain the old value).  
+        if (item.CanonicalName is not string currentFilePath)
             return null;
 
         var source = new RootSymbolTreeItemCollectionSource(this, item);
-        _hierarchyToCollectionSource[item] = source;
+        lock (_filePathToCollectionSources)
+            _filePathToCollectionSources.Add(currentFilePath, source);
 
         // Register to hear about if this hierarchy is disposed. We'll stop watching it if so.
         item.PropertyChanged += OnItemPropertyChanged;
@@ -166,18 +178,37 @@ void OnItemPropertyChanged(object sender, PropertyChangedEventArgs e)
             if (e.PropertyName == nameof(ISupportDisposalNotification.IsDisposed) && item.IsDisposed)
             {
                 // We are notified when the IVsHierarchyItem is removed from the tree via its INotifyPropertyChanged
-                // event for the IsDisposed property. When this fires, we remove the item->sourcce mapping we're holding.
-                _hierarchyToCollectionSource.TryRemove(item, out _);
+                // event for the IsDisposed property. When this fires, we remove the filePath->sourcce mapping we're holding.
+                lock (_filePathToCollectionSources)
+                {
+                    _filePathToCollectionSources.Remove(currentFilePath, source);
+                }
+
                 item.PropertyChanged -= OnItemPropertyChanged;
             }
             else if (e.PropertyName == nameof(IVsHierarchyItem.CanonicalName))
             {
-                // If the filepath changes for an item (which can happen when it is renamed), place a notification
-                // in the queue to update it in the future.  This will ensure all the items presented for it have hte
-                // right document id.  Also reset the state of the source.  The filepath could change to something
-                // no longer valid (like .cs to .txt), or vice versa.
-                source.Reset();
-                _updateSourcesQueue.AddWork(item.CanonicalName);
+                var newPath = item.CanonicalName;
+                if (newPath != currentFilePath)
+                {
+                    lock (_filePathToCollectionSources)
+                    {
+
+                        // Unlink the oldPath->source mapping, and add a new line for the newPath->source.
+                        _filePathToCollectionSources.Remove(currentFilePath, source);
+                        _filePathToCollectionSources.Add(newPath, source);
+
+                        // Keep track of the 'newPath'.
+                        currentFilePath = newPath;
+                    }
+
+                    // If the filepath changes for an item (which can happen when it is renamed), place a notification
+                    // in the queue to update it in the future.  This will ensure all the items presented for it have hte
+                    // right document id.  Also reset the state of the source.  The filepath could change to something
+                    // no longer valid (like .cs to .txt), or vice versa.
+                    source.Reset();
+                    _updateSourcesQueue.AddWork(newPath);
+                }
             }
         }
     }