Merge pull request #32649 from...

Merge pull request #32649 from jasonmalinowski/fix-metadata-references-not-always-having-file-watchers

Fix metadata references not always having file watchers

src/VisualStudio/Core/Def/Implementation/ProjectSystem/FileChangeWatcherProvider.cs

 using System;
 using System.ComponentModel.Composition;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
+using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Shell.Interop;
 
 namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
@@ -9,18 +11,23 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
     internal sealed class FileChangeWatcherProvider
     {
         private readonly TaskCompletionSource<IVsFileChangeEx> _fileChangeService = new TaskCompletionSource<IVsFileChangeEx>(TaskCreationOptions.RunContinuationsAsynchronously);
-        private readonly Lazy<FileChangeWatcher> _fileChangeWatcher;
 
-        public FileChangeWatcherProvider()
+        [ImportingConstructor]
+        public FileChangeWatcherProvider(IThreadingContext threadingContext, [Import(typeof(SVsServiceProvider))] Shell.IAsyncServiceProvider serviceProvider)
         {
-            _fileChangeWatcher = new Lazy<FileChangeWatcher>(() => new FileChangeWatcher(_fileChangeService.Task));
-        }
+            // We do not want background work to implicitly block on the availability of the SVsFileChangeEx to avoid any deadlock risk,
+            // since the first fetch for a file watcher might end up happening on the background.
+            Watcher = new FileChangeWatcher(_fileChangeService.Task);
 
-        public FileChangeWatcher Watcher => _fileChangeWatcher.Value;
+            System.Threading.Tasks.Task.Run(async () =>
+                {
+                    await threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync();
 
-        internal void SetFileChangeService(IVsFileChangeEx fileChangeService)
-        {
-            _fileChangeService.TrySetResult(fileChangeService);
+                    var fileChangeService = (IVsFileChangeEx)await serviceProvider.GetServiceAsync(typeof(SVsFileChangeEx)).ConfigureAwait(true);
+                    _fileChangeService.SetResult(fileChangeService);
+                });
         }
+
+        public FileChangeWatcher Watcher { get; }
     }
 }

src/VisualStudio/Core/Def/Implementation/ProjectSystem/MetadataReferences/FileWatchedPortableExecutableReferenceFactory.cs

+using System;
+using System.Collections.Generic;
+using System.ComponentModel.Composition;
+using System.Linq;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+
+namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.MetadataReferences
+{
+    [Export]
+    internal sealed class FileWatchedPortableExecutableReferenceFactory
+    {
+        private readonly object _gate = new object();
+
+        /// <summary>
+        /// This right now acquires the entire VisualStudioWorkspace because right now the production
+        /// of metadata references depends on other workspace services. See the comments on
+        /// <see cref="VisualStudioMetadataReferenceManagerFactory"/> that this strictly shouldn't be necessary
+        /// but for now is quite the tangle to fix.
+        /// </summary>
+        private readonly Lazy<VisualStudioWorkspace> _visualStudioWorkspace;
+
+        /// <summary>
+        /// A file change context used to watch metadata references.
+        /// </summary>
+        private readonly FileChangeWatcher.IContext _fileReferenceChangeContext;
+
+        /// <summary>
+        /// File watching tokens from <see cref="_fileReferenceChangeContext"/> that are watching metadata references. These are only created once we are actually applying a batch because
+        /// we don't determine until the batch is applied if the file reference will actually be a file reference or it'll be a converted project reference.
+        /// </summary>
+        private readonly Dictionary<PortableExecutableReference, FileChangeWatcher.IFileWatchingToken> _metadataReferenceFileWatchingTokens = new Dictionary<PortableExecutableReference, FileChangeWatcher.IFileWatchingToken>();
+
+        /// <summary>
+        /// <see cref="CancellationTokenSource"/>s for in-flight refreshing of metadata references. When we see a file change, we wait a bit before trying to actually
+        /// update the workspace. We need cancellation tokens for those so we can cancel them either when a flurry of events come in (so we only do the delay after the last
+        /// modification), or when we know the project is going away entirely.
+        /// </summary>
+        private readonly Dictionary<string, CancellationTokenSource> _metadataReferenceRefreshCancellationTokenSources = new Dictionary<string, CancellationTokenSource>();
+
+        [ImportingConstructor]
+        public FileWatchedPortableExecutableReferenceFactory(
+            Lazy<VisualStudioWorkspace> visualStudioWorkspace,
+            FileChangeWatcherProvider fileChangeWatcherProvider)
+        {
+            _visualStudioWorkspace = visualStudioWorkspace;
+
+            // TODO: set this to watch the NuGet directory or the reference assemblies directory; since those change rarely and most references
+            // will come from them, we can avoid creating a bunch of explicit file watchers.
+            _fileReferenceChangeContext = fileChangeWatcherProvider.Watcher.CreateContext();
+            _fileReferenceChangeContext.FileChanged += FileReferenceChangeContext_FileChanged;
+        }
+
+        public event EventHandler<string> ReferenceChanged;
+
+        public PortableExecutableReference CreateReferenceAndStartWatchingFile(string fullFilePath, MetadataReferenceProperties properties)
+        {
+            lock (_gate)
+            {
+                var reference = _visualStudioWorkspace.Value.CreatePortableExecutableReference(fullFilePath, properties);
+                var fileWatchingToken = _fileReferenceChangeContext.EnqueueWatchingFile(fullFilePath);
+
+                _metadataReferenceFileWatchingTokens.Add(reference, fileWatchingToken);
+
+                return reference;
+            }
+        }
+
+        public void StopWatchingReference(PortableExecutableReference reference)
+        {
+            lock (_gate)
+            {
+                if (!_metadataReferenceFileWatchingTokens.TryGetValue(reference, out var token))
+                {
+                    throw new ArgumentException("The reference was already not being watched.");
+                }
+
+                _fileReferenceChangeContext.StopWatchingFile(token);
+                _metadataReferenceFileWatchingTokens.Remove(reference);
+
+                // Note we still potentially have an outstanding change that we haven't raised a notification
+                // for due to the delay we use. We could cancel the notification for that file path,
+                // but we may still have another outstanding PortableExecutableReference that isn't this one
+                // that does want that notification. We're OK just leaving the delay still running for two
+                // reasons:
+                //
+                // 1. Technically, we did see a file change before the call to StopWatchingReference, so
+                //    arguably we should still raise it.
+                // 2. Since we raise the notification for a file path, it's up to the consumer of this to still
+                //    track down which actual reference needs to be changed. That'll automatically handle any
+                //    race where the event comes late, which is a scenario this must always deal with no matter
+                //    what -- another thread might already be gearing up to notify the caller of this reference
+                //    and we can't stop it.
+            }
+        }
+
+        private void FileReferenceChangeContext_FileChanged(object sender, string fullFilePath)
+        {
+            lock (_gate)
+            {
+                if (_metadataReferenceRefreshCancellationTokenSources.TryGetValue(fullFilePath, out var cancellationTokenSource))
+                {
+                    cancellationTokenSource.Cancel();
+                    _metadataReferenceRefreshCancellationTokenSources.Remove(fullFilePath);
+                }
+
+                cancellationTokenSource = new CancellationTokenSource();
+                _metadataReferenceRefreshCancellationTokenSources.Add(fullFilePath, cancellationTokenSource);
+
+                Task.Delay(TimeSpan.FromSeconds(5), cancellationTokenSource.Token).ContinueWith(_ =>
+                {
+                    bool needsNotification = false;
+
+                    lock (_gate)
+                    {
+                        // We need to re-check the cancellation token source under the lock, since it might have been cancelled and restarted
+                        // due to another event
+                        cancellationTokenSource.Token.ThrowIfCancellationRequested();
+                        needsNotification = true;
+
+                        _metadataReferenceRefreshCancellationTokenSources.Remove(fullFilePath);
+                    }
+
+                    if (needsNotification)
+                    {
+                        ReferenceChanged?.Invoke(this, fullFilePath);
+                    }
+                }, cancellationTokenSource.Token, TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.Default);
+            }
+        }
+    }
+}

src/VisualStudio/Core/Def/Implementation/ProjectSystem/MetadataReferences/VisualStudioMetadataReference.Snapshot.cs

-// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
-
-using System;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.IO;
-using System.Threading;
-using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.Execution;
-using Microsoft.CodeAnalysis.Host;
-using Microsoft.VisualStudio.LanguageServices.Implementation.DocumentationComments;
-using Roslyn.Utilities;
-
-namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
-{
-    // TODO: This class is now an empty container just to hold onto the nested type. Renaming that is an invasive change that will be it's own commit.
-    internal static class VisualStudioMetadataReference
-    {
-        /// <summary>
-        /// Represents a metadata reference corresponding to a specific version of a file.
-        /// If a file changes in future this reference will still refer to the original version.
-        /// </summary>
-        /// <remarks>
-        /// The compiler observes the metadata content a reference refers to by calling <see cref="PortableExecutableReference.GetMetadataImpl()"/>
-        /// and the observed metadata is memoized by the compilation. However we drop compilations to decrease memory consumption. 
-        /// When the compilation is recreated for a solution the compiler asks for metadata again and we need to provide the original content,
-        /// not read the file again. Therefore we need to save the timestamp on the <see cref="Snapshot"/>.
-        /// 
-        /// When the VS observes a change in a metadata reference file the project version is advanced and a new instance of 
-        /// <see cref="Snapshot"/> is created for the corresponding reference.
-        /// </remarks>
-        [DebuggerDisplay("{GetDebuggerDisplay(),nq}")]
-        internal sealed class Snapshot : PortableExecutableReference, ISupportTemporaryStorage
-        {
-            private readonly VisualStudioMetadataReferenceManager _provider;
-            private readonly Lazy<DateTime> _timestamp;
-            private Exception _error;
-            private FileChangeTracker _fileChangeTrackerOpt;
-
-            internal Snapshot(VisualStudioMetadataReferenceManager provider, MetadataReferenceProperties properties, string fullPath, FileChangeTracker fileChangeTrackerOpt)
-                : base(properties, fullPath)
-            {
-                Debug.Assert(Properties.Kind == MetadataImageKind.Assembly);
-                _provider = provider;
-                _fileChangeTrackerOpt = fileChangeTrackerOpt;
-
-                _timestamp = new Lazy<DateTime>(() =>
-                {
-                    try
-                    {
-                        _fileChangeTrackerOpt?.EnsureSubscription();
-
-                        return FileUtilities.GetFileTimeStamp(this.FilePath);
-                    }
-                    catch (IOException e)
-                    {
-                        // Reading timestamp of a file might fail. 
-                        // Let's remember the failure and report it to the compiler when it asks for metadata.
-                        // We could let the Lazy hold onto this (since it knows how to rethrow exceptions), but
-                        // our support of GetStorages needs to gracefully handle the case where we have no timestamp.
-                        // If Lazy had a "IsValueFaulted" we could be cleaner here.
-                        _error = e;
-                        return DateTime.MinValue;
-                    }
-                }, LazyThreadSafetyMode.PublicationOnly);
-            }
-
-            protected override Metadata GetMetadataImpl()
-            {
-                // Fetch the timestamp first, so as to populate _error if needed
-                var timestamp = _timestamp.Value;
-
-                if (_error != null)
-                {
-                    throw _error;
-                }
-
-                try
-                {
-                    return _provider.GetMetadata(this.FilePath, timestamp);
-                }
-                catch (Exception e) when (SaveMetadataReadingException(e))
-                {
-                    throw ExceptionUtilities.Unreachable;
-                }
-            }
-
-            private bool SaveMetadataReadingException(Exception e)
-            {
-                // Save metadata reading failure so that future compilations created 
-                // with this reference snapshot fail consistently in the same way.
-                if (e is IOException || e is BadImageFormatException)
-                {
-                    _error = e;
-                }
-
-                return false;
-            }
-
-            protected override DocumentationProvider CreateDocumentationProvider()
-            {
-                return new VisualStudioDocumentationProvider(this.FilePath, _provider.XmlMemberIndexService);
-            }
-
-            protected override PortableExecutableReference WithPropertiesImpl(MetadataReferenceProperties properties)
-            {
-                return new Snapshot(_provider, properties, this.FilePath, _fileChangeTrackerOpt);
-            }
-
-            private string GetDebuggerDisplay()
-            {
-                return "Metadata File: " + FilePath;
-            }
-
-            public IEnumerable<ITemporaryStreamStorage> GetStorages()
-            {
-                return _provider.GetStorages(this.FilePath, _timestamp.Value);
-            }
-        }
-    }
-}

src/VisualStudio/Core/Def/Implementation/ProjectSystem/MetadataReferences/VisualStudioMetadataReferenceManager.cs

@@ -81,9 +81,9 @@ internal IEnumerable<ITemporaryStreamStorage> GetStorages(string fullPath, DateT
             return null;
         }
 
-        public PortableExecutableReference CreateMetadataReferenceSnapshot(string filePath, MetadataReferenceProperties properties)
+        public PortableExecutableReference CreateMetadataReference(string filePath, MetadataReferenceProperties properties)
         {
-            return new VisualStudioMetadataReference.Snapshot(this, properties, filePath, fileChangeTrackerOpt: null);
+            return new VisualStudioPortableExecutableReference(this, properties, filePath, fileChangeTrackerOpt: null);
         }
 
         public void ClearCache()

src/VisualStudio/Core/Def/Implementation/ProjectSystem/MetadataReferences/VisualStudioMetadataReferenceProviderServiceFactory.cs

@@ -32,7 +32,7 @@ public Service(HostWorkspaceServices workspaceServices)
 
             public PortableExecutableReference GetReference(string resolvedPath, MetadataReferenceProperties properties)
             {
-                return _manager.Value.CreateMetadataReferenceSnapshot(resolvedPath, properties);
+                return _manager.Value.CreateMetadataReference(resolvedPath, properties);
             }
         }
     }

src/VisualStudio/Core/Def/Implementation/ProjectSystem/MetadataReferences/VisualStudioPortableExecutableReference.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.IO;
+using System.Threading;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Execution;
+using Microsoft.CodeAnalysis.Host;
+using Microsoft.VisualStudio.LanguageServices.Implementation.DocumentationComments;
+using Roslyn.Utilities;
+
+namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
+{
+    /// <summary>
+    /// Represents a metadata reference corresponding to a specific version of a file.
+    /// If a file changes in future this reference will still refer to the original version.
+    /// </summary>
+    /// <remarks>
+    /// The compiler observes the metadata content a reference refers to by calling <see cref="PortableExecutableReference.GetMetadataImpl()"/>
+    /// and the observed metadata is memoized by the compilation. However we drop compilations to decrease memory consumption. 
+    /// When the compilation is recreated for a solution the compiler asks for metadata again and we need to provide the original content,
+    /// not read the file again. Therefore we need to save the timestamp on the <see cref="VisualStudioPortableExecutableReference"/>.
+    /// 
+    /// When the VS observes a change in a metadata reference file the project version is advanced and a new instance of 
+    /// <see cref="VisualStudioPortableExecutableReference"/> is created for the corresponding reference.
+    /// </remarks>
+    [DebuggerDisplay("{GetDebuggerDisplay(),nq}")]
+    internal sealed class VisualStudioPortableExecutableReference : PortableExecutableReference, ISupportTemporaryStorage
+    {
+        private readonly VisualStudioMetadataReferenceManager _referenceManager;
+        private readonly Lazy<DateTime> _timestamp;
+        private Exception _error;
+        private FileChangeTracker _fileChangeTrackerOpt;
+
+        internal VisualStudioPortableExecutableReference(VisualStudioMetadataReferenceManager referenceManager, MetadataReferenceProperties properties, string fullPath, FileChangeTracker fileChangeTrackerOpt)
+            : base(properties, fullPath)
+        {
+            Debug.Assert(Properties.Kind == MetadataImageKind.Assembly);
+            _referenceManager = referenceManager;
+            _fileChangeTrackerOpt = fileChangeTrackerOpt;
+
+            _timestamp = new Lazy<DateTime>(() =>
+            {
+                try
+                {
+                    _fileChangeTrackerOpt?.EnsureSubscription();
+
+                    return FileUtilities.GetFileTimeStamp(this.FilePath);
+                }
+                catch (IOException e)
+                {
+                    // Reading timestamp of a file might fail. 
+                    // Let's remember the failure and report it to the compiler when it asks for metadata.
+                    // We could let the Lazy hold onto this (since it knows how to rethrow exceptions), but
+                    // our support of GetStorages needs to gracefully handle the case where we have no timestamp.
+                    // If Lazy had a "IsValueFaulted" we could be cleaner here.
+                    _error = e;
+                    return DateTime.MinValue;
+                }
+            }, LazyThreadSafetyMode.PublicationOnly);
+        }
+
+        protected override Metadata GetMetadataImpl()
+        {
+            // Fetch the timestamp first, so as to populate _error if needed
+            var timestamp = _timestamp.Value;
+
+            if (_error != null)
+            {
+                throw _error;
+            }
+
+            try
+            {
+                return _referenceManager.GetMetadata(this.FilePath, timestamp);
+            }
+            catch (Exception e) when (SaveMetadataReadingException(e))
+            {
+                throw ExceptionUtilities.Unreachable;
+            }
+        }
+
+        private bool SaveMetadataReadingException(Exception e)
+        {
+            // Save metadata reading failure so that future compilations created 
+            // with this reference snapshot fail consistently in the same way.
+            if (e is IOException || e is BadImageFormatException)
+            {
+                _error = e;
+            }
+
+            return false;
+        }
+
+        protected override DocumentationProvider CreateDocumentationProvider()
+        {
+            return new VisualStudioDocumentationProvider(this.FilePath, _referenceManager.XmlMemberIndexService);
+        }
+
+        protected override PortableExecutableReference WithPropertiesImpl(MetadataReferenceProperties properties)
+        {
+            return new VisualStudioPortableExecutableReference(_referenceManager, properties, this.FilePath, _fileChangeTrackerOpt);
+        }
+
+        private string GetDebuggerDisplay()
+        {
+            return "Metadata File: " + FilePath;
+        }
+
+        public IEnumerable<ITemporaryStreamStorage> GetStorages()
+        {
+            return _referenceManager.GetStorages(this.FilePath, _timestamp.Value);
+        }
+    }
+}

src/VisualStudio/Core/Def/Implementation/ProjectSystem/MiscellaneousFilesWorkspace.cs

@@ -112,7 +112,7 @@ private IEnumerable<MetadataReference> CreateMetadataReferences()
             return from fileName in new[] { "mscorlib.dll", "System.dll", "System.Core.dll" }
                    let fullPath = FileUtilities.ResolveRelativePath(fileName, basePath: null, baseDirectory: null, searchPaths: searchPaths, fileExists: File.Exists)
                    where fullPath != null
-                   select manager.CreateMetadataReferenceSnapshot(fullPath, MetadataReferenceProperties.Assembly);
+                   select manager.CreateMetadataReference(fullPath, MetadataReferenceProperties.Assembly);
         }
 
         public int OnAfterAttributeChange(uint docCookie, uint grfAttribs)

src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs

@@ -73,24 +73,6 @@ internal sealed class VisualStudioProject
         /// </summary>
         private readonly FileChangeWatcher.IContext _documentFileChangeContext;
 
-        /// <summary>
-        /// A file change context used to watch metadata and analyzer references.
-        /// </summary>
-        private readonly FileChangeWatcher.IContext _fileReferenceChangeContext;
-
-        /// <summary>
-        /// File watching tokens from <see cref="_fileReferenceChangeContext"/> that are watching metadata references. These are only created once we are actually applying a batch because
-        /// we don't determine until the batch is applied if the file reference will actually be a file reference or it'll be a converted project reference.
-        /// </summary>
-        private readonly Dictionary<PortableExecutableReference, FileChangeWatcher.IFileWatchingToken> _metadataReferenceFileWatchingTokens = new Dictionary<PortableExecutableReference, FileChangeWatcher.IFileWatchingToken>();
-
-        /// <summary>
-        /// <see cref="CancellationTokenSource"/>s for in-flight refreshing of metadata references. When we see a file change, we wait a bit before trying to actually
-        /// update the workspace. We need cancellation tokens for those so we can cancel them either when a flurry of events come in (so we only do the delay after the last
-        /// modification), or when we know the project is going away entirely. We don't 
-        /// </summary>
-        private readonly Dictionary<string, CancellationTokenSource> _metadataReferenceRefreshCancellationTokenSources = new Dictionary<string, CancellationTokenSource>();
-
         /// <summary>
         /// track whether we have been subscribed to <see cref="IDynamicFileInfoProvider.Updated"/> event
         /// </summary>
@@ -143,11 +125,6 @@ internal sealed class VisualStudioProject
 
             _documentFileChangeContext.FileChanged += DocumentFileChangeContext_FileChanged;
 
-            // TODO: set this to watch the NuGet directory or the reference assemblies directory; since those change rarely and most references
-            // will come from them, we can avoid creating a bunch of explicit file watchers.
-            _fileReferenceChangeContext = workspace.FileChangeWatcher.CreateContext();
-            _fileReferenceChangeContext.FileChanged += FileReferenceChangeContext_FileChanged;
-
             _sourceFiles = new BatchingDocumentCollection(
                 this,
                 documentAlreadyInWorkspace: (s, d) => s.ContainsDocument(d),
@@ -424,9 +401,8 @@ private void OnBatchScopeDisposed()
                             }
                             else
                             {
-                                var metadataReference = _workspace.CreatePortableExecutableReference(metadataReferenceAddedInBatch.path, metadataReferenceAddedInBatch.properties);
+                                var metadataReference = _workspace.FileWatchedReferenceFactory.CreateReferenceAndStartWatchingFile(metadataReferenceAddedInBatch.path, metadataReferenceAddedInBatch.properties);
                                 metadataReferencesCreated.Add(metadataReference);
-                                _metadataReferenceFileWatchingTokens.Add(metadataReference, _fileReferenceChangeContext.EnqueueWatchingFile(metadataReference.FilePath));
                             }
                         }
 
@@ -451,9 +427,7 @@ private void OnBatchScopeDisposed()
                             var metadataReference = _workspace.CurrentSolution.GetProject(Id).MetadataReferences.Cast<PortableExecutableReference>()
                                                                                     .Single(m => m.FilePath == metadataReferenceRemovedInBatch.path && m.Properties == metadataReferenceRemovedInBatch.properties);
 
-                            _fileReferenceChangeContext.StopWatchingFile(_metadataReferenceFileWatchingTokens[metadataReference]);
-                            _metadataReferenceFileWatchingTokens.Remove(metadataReference);
-                            CancelOutstandingMetadataReferenceRefreshForFile_NoLock(metadataReference.FilePath);
+                            _workspace.FileWatchedReferenceFactory.StopWatchingReference(metadataReference);
 
                             solution = solution.RemoveMetadataReference(Id, metadataReference);
                         }
@@ -707,75 +681,6 @@ private void DocumentFileChangeContext_FileChanged(object sender, string fullFil
             _additionalFiles.ProcessFileChange(fullFilePath);
         }
 
-        #region Metadata Reference Refreshing
-
-        private void FileReferenceChangeContext_FileChanged(object sender, string fullFilePath)
-        {
-            lock (_gate)
-            {
-                CancelOutstandingMetadataReferenceRefreshForFile_NoLock(fullFilePath);
-
-                var cancellationTokenSource = new CancellationTokenSource();
-                _metadataReferenceRefreshCancellationTokenSources.Add(fullFilePath, cancellationTokenSource);
-
-                Task.Delay(TimeSpan.FromSeconds(5), cancellationTokenSource.Token).ContinueWith(_ =>
-                {
-                    lock (_gate)
-                    {
-                        // We need to re-check the cancellation token source under the lock, since it might have been cancelled and restarted
-                        // due to another event
-                        cancellationTokenSource.Token.ThrowIfCancellationRequested();
-                        RefreshMetadataReferencesForFile_NoLock(fullFilePath);
-
-                        _metadataReferenceRefreshCancellationTokenSources.Remove(fullFilePath);
-                    }
-                }, cancellationTokenSource.Token, TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.Default);
-            }
-        }
-
-        private void RefreshMetadataReferencesForFile_NoLock(string fullFilePath)
-        {
-            // Since all adds/removals of references for this project happen under our lock, it's safe to do this
-            // check without taking the main workspace lock.
-            var project = _workspace.CurrentSolution.GetProject(Id);
-
-            foreach (var portableExecutableReference in project.MetadataReferences.OfType<PortableExecutableReference>())
-            {
-                // Loop to find each reference with the given path. It's possible that there might be multiple references of the same path;
-                // the project system could concievably add the same reference multiple times but with different aliases. It's also possible
-                // we might not find the path at all: when we recieve the file changed event, we aren't checking if the file is still
-                // in the workspace at that time; it's possible it might have already been removed. We could add a second check for the file
-                // there, but it's just overhead checking for a rare situation we'll still be able to deal with here.
-                if (portableExecutableReference.FilePath == fullFilePath)
-                {
-                    var newPortableExecutableReference = _workspace.CreatePortableExecutableReference(portableExecutableReference.FilePath, portableExecutableReference.Properties);
-
-                    // We need to swap this out. Time to take the full lock now.
-                    _workspace.ApplyBatchChangeToProject(Id, s =>
-                    {
-                        return s.RemoveMetadataReference(Id, portableExecutableReference)
-                                .AddMetadataReference(Id, newPortableExecutableReference);
-                    });
-
-                    // Transfer the ownership of the file watching token
-                    var fileWatchingToken = _metadataReferenceFileWatchingTokens[portableExecutableReference];
-                    _metadataReferenceFileWatchingTokens.Remove(portableExecutableReference);
-                    _metadataReferenceFileWatchingTokens.Add(newPortableExecutableReference, fileWatchingToken);
-                }
-            }
-        }
-
-        private void CancelOutstandingMetadataReferenceRefreshForFile_NoLock(string fullFilePath)
-        {
-            if (_metadataReferenceRefreshCancellationTokenSources.TryGetValue(fullFilePath, out var cancellationTokenSource))
-            {
-                cancellationTokenSource.Cancel();
-                _metadataReferenceRefreshCancellationTokenSources.Remove(fullFilePath);
-            }
-        }
-
-        #endregion
-
         #region Metadata Reference Addition/Removal
 
         public void AddMetadataReference(string fullPath, MetadataReferenceProperties properties)
@@ -813,9 +718,8 @@ public void AddMetadataReference(string fullPath, MetadataReferenceProperties pr
                         }
                         else
                         {
-                            var metadataReference = _workspace.CreatePortableExecutableReference(fullPath, properties);
+                            var metadataReference = _workspace.FileWatchedReferenceFactory.CreateReferenceAndStartWatchingFile(fullPath, properties);
                             w.OnMetadataReferenceAdded(Id, metadataReference);
-                            _metadataReferenceFileWatchingTokens.Add(metadataReference, _fileReferenceChangeContext.EnqueueWatchingFile(metadataReference.FilePath));
                         }
                     });
                 }
@@ -885,11 +789,8 @@ public void RemoveMetadataReference(string fullPath, MetadataReferenceProperties
                             var metadataReference = w.CurrentSolution.GetProject(Id).MetadataReferences.Cast<PortableExecutableReference>()
                                                                                     .Single(m => m.FilePath == fullPath && m.Properties == properties);
 
+                            _workspace.FileWatchedReferenceFactory.StopWatchingReference(metadataReference);
                             w.OnMetadataReferenceRemoved(Id, metadataReference);
-
-                            _fileReferenceChangeContext.StopWatchingFile(_metadataReferenceFileWatchingTokens[metadataReference]);
-                            _metadataReferenceFileWatchingTokens.Remove(metadataReference);
-                            CancelOutstandingMetadataReferenceRefreshForFile_NoLock(metadataReference.FilePath);
                         }
                     });
                 }
@@ -1000,7 +901,6 @@ public void RemoveProjectReference(ProjectReference projectReference)
         public void RemoveFromWorkspace()
         {
             _documentFileChangeContext.Dispose();
-            _fileReferenceChangeContext.Dispose();
 
             lock (_gate)
             {
@@ -1012,10 +912,10 @@ public void RemoveFromWorkspace()
 
                 _eventSubscriptionTracker.Clear();
 
-                // Clear any remaining pending refreshes we have for files
-                foreach (var cancellationTokenSource in _metadataReferenceRefreshCancellationTokenSources.Values)
+                // Clear any file watchers we still have for references
+                foreach (PortableExecutableReference reference in _workspace.CurrentSolution.GetProject(Id).MetadataReferences)
                 {
-                    cancellationTokenSource.Cancel();
+                    _workspace.FileWatchedReferenceFactory.StopWatchingReference(reference);
                 }
             }
 

src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs

@@ -3,6 +3,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -12,7 +13,7 @@
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
 using Microsoft.CodeAnalysis.Host;
-using Microsoft.CodeAnalysis.Host.Mef;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Utilities;
 using Microsoft.CodeAnalysis.SolutionCrawler;
 using Microsoft.CodeAnalysis.Text;
@@ -20,6 +21,7 @@
 using Microsoft.VisualStudio.Composition;
 using Microsoft.VisualStudio.Editor;
 using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.Extensions;
+using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.MetadataReferences;
 using Microsoft.VisualStudio.LanguageServices.Implementation.Venus;
 using Microsoft.VisualStudio.LanguageServices.Utilities;
 using Microsoft.VisualStudio.Shell;
@@ -73,6 +75,7 @@ internal abstract partial class VisualStudioWorkspaceImpl : VisualStudioWorkspac
 
         private OpenFileTracker _openFileTrackerOpt;
         internal FileChangeWatcher FileChangeWatcher { get; }
+        internal FileWatchedPortableExecutableReferenceFactory FileWatchedReferenceFactory { get; }
 
         public VisualStudioWorkspaceImpl(ExportProvider exportProvider, IAsyncServiceProvider asyncServiceProvider)
             : base(VisualStudioMefHostServices.Create(exportProvider))
@@ -96,16 +99,10 @@ public VisualStudioWorkspaceImpl(ExportProvider exportProvider, IAsyncServicePro
 
             System.Threading.Tasks.Task.Run(() => ConnectToOpenFileTrackerOnUIThreadAsync(asyncServiceProvider));
 
-            var fileChangeWatcherProvider = exportProvider.GetExportedValue<FileChangeWatcherProvider>();
+            FileChangeWatcher = exportProvider.GetExportedValue<FileChangeWatcherProvider>().Watcher;
+            FileWatchedReferenceFactory = exportProvider.GetExportedValue<FileWatchedPortableExecutableReferenceFactory>();
 
-            FileChangeWatcher = fileChangeWatcherProvider.Watcher;
-            System.Threading.Tasks.Task.Run(async () =>
-                {
-                    await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync();
-
-                    var fileChangeService = (IVsFileChangeEx)ServiceProvider.GlobalProvider.GetService(typeof(SVsFileChangeEx));
-                    fileChangeWatcherProvider.SetFileChangeService(fileChangeService);
-                });
+            FileWatchedReferenceFactory.ReferenceChanged += this.RefreshMetadataReferencesForFile;
         }
 
         public async System.Threading.Tasks.Task ConnectToOpenFileTrackerOnUIThreadAsync(IAsyncServiceProvider asyncServiceProvider)
@@ -1173,17 +1170,12 @@ protected override void Dispose(bool finalize)
             {
                 _textBufferFactoryService.TextBufferCreated -= AddTextBufferCloneServiceToBuffer;
                 _projectionBufferFactoryService.ProjectionBufferCreated -= AddTextBufferCloneServiceToBuffer;
+                FileWatchedReferenceFactory.ReferenceChanged -= RefreshMetadataReferencesForFile;
             }
 
             // workspace is going away. unregister this workspace from work coordinator
             StopSolutionCrawler();
 
-            // We should consider calling this here. It is commented out because Solution event tracking was
-            // moved from VisualStudioProjectTracker, which is never Dispose()'d.  Rather than risk the
-            // UnadviseSolutionEvents causing another issue (calling into dead COM objects, etc), we'll just
-            // continue to skip it for now.
-            // UnadviseSolutionEvents();
-
             base.Dispose(finalize);
         }
 
@@ -1430,7 +1422,7 @@ public void AddProjectOutputPath(ProjectId projectId, string outputPath)
         private void ConvertMetadataReferencesToProjectReferences_NoLock(ProjectId projectId, string outputPath)
         {
             var modifiedSolution = this.CurrentSolution;
-            var projectIdsChanged = new HashSet<ProjectId>();
+            var projectIdsChanged = PooledHashSet<ProjectId>.GetInstance();
 
             foreach (var projectIdToRetarget in this.CurrentSolution.ProjectIds)
             {
@@ -1442,6 +1434,8 @@ private void ConvertMetadataReferencesToProjectReferences_NoLock(ProjectId proje
                     {
                         if (string.Equals(reference.FilePath, outputPath, StringComparison.OrdinalIgnoreCase))
                         {
+                            FileWatchedReferenceFactory.StopWatchingReference(reference);
+
                             var projectReference = new ProjectReference(projectId, reference.Properties.Aliases, reference.Properties.EmbedInteropTypes);
                             modifiedSolution = modifiedSolution.RemoveMetadataReference(projectIdToRetarget, reference)
                                                                .AddProjectReference(projectIdToRetarget, projectReference);
@@ -1459,6 +1453,7 @@ private void ConvertMetadataReferencesToProjectReferences_NoLock(ProjectId proje
             }
 
             SetSolutionAndRaiseWorkspaceChanged_NoLock(modifiedSolution, projectIdsChanged);
+            projectIdsChanged.Free();
         }
 
         [PerformanceSensitive("https://github.com/dotnet/roslyn/issues/31306",
@@ -1496,7 +1491,7 @@ private bool CanConvertMetadataReferenceToProjectReference(ProjectId projectIdWi
         private void ConvertProjectReferencesToMetadataReferences_NoLock(ProjectId projectId, string outputPath)
         {
             var modifiedSolution = this.CurrentSolution;
-            var projectIdsChanged = new HashSet<ProjectId>();
+            var projectIdsChanged = PooledHashSet<ProjectId>.GetInstance();
 
             foreach (var projectIdToRetarget in this.CurrentSolution.ProjectIds)
             {
@@ -1508,7 +1503,7 @@ private void ConvertProjectReferencesToMetadataReferences_NoLock(ProjectId proje
                         convertedReference.projectReference.ProjectId == projectId)
                     {
                         var metadataReference =
-                            CreatePortableExecutableReference(
+                            FileWatchedReferenceFactory.CreateReferenceAndStartWatchingFile(
                                 convertedReference.path,
                                 new MetadataReferenceProperties(
                                     aliases: convertedReference.projectReference.Aliases,
@@ -1528,6 +1523,7 @@ private void ConvertProjectReferencesToMetadataReferences_NoLock(ProjectId proje
             }
 
             SetSolutionAndRaiseWorkspaceChanged_NoLock(modifiedSolution, projectIdsChanged);
+            projectIdsChanged.Free();
         }
 
         public ProjectReference TryCreateConvertedProjectReference(ProjectId referencingProject, string path, MetadataReferenceProperties properties)
@@ -1585,6 +1581,8 @@ private void SetSolutionAndRaiseWorkspaceChanged_NoLock(CodeAnalysis.Solution mo
         {
             if (projectIdsChanged.Count > 0)
             {
+                Debug.Assert(modifiedSolution != CurrentSolution);
+
                 var originalSolution = this.CurrentSolution;
                 SetCurrentSolution(modifiedSolution);
 
@@ -1597,6 +1595,11 @@ private void SetSolutionAndRaiseWorkspaceChanged_NoLock(CodeAnalysis.Solution mo
                     RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind.SolutionChanged, originalSolution, this.CurrentSolution);
                 }
             }
+            else
+            {
+                // If they said nothing changed, than definitely nothing should have changed!
+                Debug.Assert(modifiedSolution == CurrentSolution);
+            }
         }
 
         public void RemoveProjectOutputPath(ProjectId projectId, string outputPath)
@@ -1628,5 +1631,42 @@ public void RemoveProjectOutputPath(ProjectId projectId, string outputPath)
                 }
             }
         }
+
+        private void RefreshMetadataReferencesForFile(object sender, string fullFilePath)
+        {
+            lock (_gate)
+            {
+                var newSolution = CurrentSolution;
+                var changedProjectIds = PooledHashSet<ProjectId>.GetInstance();
+
+                foreach (var project in CurrentSolution.Projects)
+                {
+                    // Loop to find each reference with the given path. It's possible that there might be multiple references of the same path;
+                    // the project system could concievably add the same reference multiple times but with different aliases. It's also possible
+                    // we might not find the path at all: when we recieve the file changed event, we aren't checking if the file is still
+                    // in the workspace at that time; it's possible it might have already been removed.
+                    foreach (var portableExecutableReference in project.MetadataReferences.OfType<PortableExecutableReference>())
+                    {
+                        if (portableExecutableReference.FilePath == fullFilePath)
+                        {
+                            FileWatchedReferenceFactory.StopWatchingReference(portableExecutableReference);
+
+                            var newPortableExecutableReference =
+                                FileWatchedReferenceFactory.CreateReferenceAndStartWatchingFile(
+                                    portableExecutableReference.FilePath,
+                                    portableExecutableReference.Properties);
+
+                            newSolution = newSolution.RemoveMetadataReference(project.Id, portableExecutableReference)
+                                                     .AddMetadataReference(project.Id, newPortableExecutableReference);
+
+                            changedProjectIds.Add(project.Id);
+                        }
+                    }
+                }
+
+                SetSolutionAndRaiseWorkspaceChanged_NoLock(newSolution, changedProjectIds);
+                changedProjectIds.Free();
+            }
+        }
     }
 }

src/VisualStudio/Core/Test/ProjectSystemShim/MetadataToProjectReferenceConversionTests.vb

+Imports Microsoft.CodeAnalysis
+Imports Microsoft.CodeAnalysis.Test.Utilities
+Imports Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework
+Imports Roslyn.Test.Utilities
+
+Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
+    <[UseExportProvider]>
+    Public Class MetadataToProjectReferenceConversionTests
+        <WpfFact>
+        <WorkItem(32554, "https://github.com/dotnet/roslyn/issues/32554")>
+        Public Sub ProjectReferenceConvertedToMetadataReferenceCanBeRemoved()
+            Using environment = New TestEnvironment()
+                Dim project1 = environment.ProjectFactory.CreateAndAddToWorkspace(
+                    "project1",
+                    LanguageNames.CSharp)
+
+                Dim project2 = environment.ProjectFactory.CreateAndAddToWorkspace(
+                    "project2",
+                    LanguageNames.CSharp)
+
+                Const ReferencePath = "C:\project1.dll"
+                project1.OutputFilePath = ReferencePath
+                project2.AddMetadataReference(ReferencePath, MetadataReferenceProperties.Assembly)
+
+                Dim getProject2 = Function() environment.Workspace.CurrentSolution.GetProject(project2.Id)
+
+                Assert.Single(getProject2().ProjectReferences)
+                Assert.Empty(getProject2().MetadataReferences)
+
+                project1.OutputFilePath = Nothing
+
+                Assert.Single(getProject2().MetadataReferences)
+                Assert.Empty(getProject2().ProjectReferences)
+
+                project2.RemoveMetadataReference(ReferencePath, MetadataReferenceProperties.Assembly)
+            End Using
+        End Sub
+    End Class
+End Namespace

src/VisualStudio/TestUtilities2/ProjectSystemShim/Framework/TestEnvironment.vb

@@ -40,6 +40,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
                 Dim catalog = TestExportProvider.EntireAssemblyCatalogWithCSharpAndVisualBasic
                 catalog = catalog.WithParts(GetType(FileChangeWatcherProvider),
                                             GetType(MockVisualStudioWorkspace),
+                                            GetType(MetadataReferences.FileWatchedPortableExecutableReferenceFactory),
                                             GetType(VisualStudioProjectFactory),
                                             GetType(MockServiceProvider),
                                             GetType(SolutionEventsBatchScopeCreator),
@@ -64,6 +65,12 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
             ServiceProvider = mockServiceProvider
         End Sub
 
+        Public ReadOnly Property ProjectFactory As VisualStudioProjectFactory
+            Get
+                Return ExportProvider.GetExportedValue(Of VisualStudioProjectFactory)
+            End Get
+        End Property
+
         <PartNotDiscoverable>
         <Export(GetType(VisualStudioWorkspace))>
         <Export(GetType(VisualStudioWorkspaceImpl))>
@@ -145,6 +152,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
             Implements Shell.IAsyncServiceProvider
 
             Private ReadOnly _exportProvider As Composition.ExportProvider
+            Private ReadOnly _fileChangeEx As MockVsFileChangeEx = New MockVsFileChangeEx
 
             Public MockMonitorSelection As IVsMonitorSelection
 
@@ -173,7 +181,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Fr
                         Return New MockVsSmartOpenScope
 
                     Case GetType(SVsFileChangeEx)
-                        Return New MockVsFileChangeEx
+                        Return _fileChangeEx
 
                     Case Else
                         Return Nothing