ProjectExternalErrorReporter: Avoid subscribing to solution build events...

ProjectExternalErrorReporter: Avoid subscribing to solution build events multiple times and leaking the object (#39389)

* Add FSharpProjectExternalErrorReporterFactory

* ProjectExternalErrorReporter: Avoid subscribing to solution build events multiple times and leaking the object

* Update F# shim

src/Tools/ExternalAccess/FSharp/Internal/VisualStudio/FSharpProjectExternalErrorReporterFactory.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 Microsoft.VisualStudio.Shell.Interop;
+using Microsoft.VisualStudio.LanguageServices;
+using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList;
+using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem;
+using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.Extensions;
+using Microsoft.VisualStudio.Shell;
+
+namespace Microsoft.CodeAnalysis.ExternalAccess.FSharp.Internal.VisualStudio
+{
+    internal static class FSharpProjectExternalErrorReporterFactory
+    {
+        public static IVsLanguageServiceBuildErrorReporter2 Create(ProjectId projectId, string errorCodePrefix, IServiceProvider serviceProvider)
+        {
+            ThreadHelper.ThrowIfNotOnUIThread();
+            var workspace = (VisualStudioWorkspaceImpl)serviceProvider.GetMefService<VisualStudioWorkspace>();
+            workspace.SubscribeExternalErrorDiagnosticUpdateSourceToSolutionBuildEvents();
+            return new ProjectExternalErrorReporter(projectId, errorCodePrefix, workspace);
+        }
+    }
+}

src/VisualStudio/Core/Def/Implementation/ProjectSystem/Legacy/AbstractLegacyProject.cs

@@ -58,10 +58,12 @@ internal abstract partial class AbstractLegacyProject : ForegroundThreadAffiniti
             ICommandLineParserService commandLineParserServiceOpt)
             : base(threadingContext)
         {
+            ThreadHelper.ThrowIfNotOnUIThread();
             Contract.ThrowIfNull(hierarchy);
 
             var componentModel = (IComponentModel)serviceProvider.GetService(typeof(SComponentModel));
             Workspace = componentModel.GetService<VisualStudioWorkspace>();
+            var workspaceImpl = (VisualStudioWorkspaceImpl)Workspace;
 
             var projectFilePath = hierarchy.TryGetProjectFilePath();
 
@@ -89,7 +91,7 @@ internal abstract partial class AbstractLegacyProject : ForegroundThreadAffiniti
                     ProjectGuid = GetProjectIDGuid(hierarchy),
                 });
 
-            ((VisualStudioWorkspaceImpl)Workspace).AddProjectRuleSetFileToInternalMaps(
+            workspaceImpl.AddProjectRuleSetFileToInternalMaps(
                 VisualStudioProject,
                 () => VisualStudioProjectOptionsProcessor.EffectiveRuleSetFilePath);
 
@@ -104,18 +106,9 @@ internal abstract partial class AbstractLegacyProject : ForegroundThreadAffiniti
             ConnectHierarchyEvents();
             RefreshBinOutputPath();
 
-            // TODO: https://github.com/dotnet/roslyn/issues/36065
-            // The ctor of ExternalErrorDiagnosticUpdateSource throws when running in tests since UIContextImpl calls:
-            //   (IVsMonitorSelection)ServiceProvider.GlobalProvider.GetService(typeof(IVsMonitorSelection))),
-            // which returns null.
-            try
-            {
-                _externalErrorReporter = new ProjectExternalErrorReporter(VisualStudioProject.Id, externalErrorReportingPrefix, (VisualStudioWorkspaceImpl)Workspace);
-            }
-            catch (Exception)
-            {
-            }
+            workspaceImpl.SubscribeExternalErrorDiagnosticUpdateSourceToSolutionBuildEvents();
 
+            _externalErrorReporter = new ProjectExternalErrorReporter(VisualStudioProject.Id, externalErrorReportingPrefix, workspaceImpl);
             _batchScopeCreator = componentModel.GetService<SolutionEventsBatchScopeCreator>();
             _batchScopeCreator.StartTrackingProject(VisualStudioProject, Hierarchy);
         }

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

@@ -39,6 +39,7 @@
 using VSLangProj140;
 using IAsyncServiceProvider = Microsoft.VisualStudio.Shell.IAsyncServiceProvider;
 using OleInterop = Microsoft.VisualStudio.OLE.Interop;
+using Task = System.Threading.Tasks.Task;
 
 namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
 {
@@ -105,8 +106,9 @@ internal abstract partial class VisualStudioWorkspaceImpl : VisualStudioWorkspac
         private readonly object _projectCompilationOutputsGuard = new object();
 
         private readonly Lazy<ExternalErrorDiagnosticUpdateSource> _lazyExternalErrorDiagnosticUpdateSource;
+        private bool _isExternalErrorDiagnosticUpdateSourceSubscribedToSolutionBuildEvents;
 
-        public VisualStudioWorkspaceImpl(ExportProvider exportProvider, IAsyncServiceProvider asyncServiceProvider, IEnumerable<CodeAnalysis.Options.IDocumentOptionsProviderFactory> documentOptionsProviderFactories)
+        public VisualStudioWorkspaceImpl(ExportProvider exportProvider, IAsyncServiceProvider asyncServiceProvider, IEnumerable<IDocumentOptionsProviderFactory> documentOptionsProviderFactories)
             : base(VisualStudioMefHostServices.Create(exportProvider))
         {
             _threadingContext = exportProvider.GetExportedValue<IThreadingContext>();
@@ -129,7 +131,7 @@ public VisualStudioWorkspaceImpl(ExportProvider exportProvider, IAsyncServicePro
             _projectionBufferFactoryService.ProjectionBufferCreated += AddTextBufferCloneServiceToBuffer;
             exportProvider.GetExportedValue<PrimaryWorkspace>().Register(this);
 
-            System.Threading.Tasks.Task.Run(() => ConnectToOpenFileTrackerOnUIThreadAsync(asyncServiceProvider));
+            _ = Task.Run(() => InitializeUIAffinitizedServicesAsync(asyncServiceProvider));
 
             FileChangeWatcher = exportProvider.GetExportedValue<FileChangeWatcherProvider>().Watcher;
             FileWatchedReferenceFactory = exportProvider.GetExportedValue<FileWatchedPortableExecutableReferenceFactory>();
@@ -146,7 +148,44 @@ public VisualStudioWorkspaceImpl(ExportProvider exportProvider, IAsyncServicePro
 
         internal ExternalErrorDiagnosticUpdateSource ExternalErrorDiagnosticUpdateSource => _lazyExternalErrorDiagnosticUpdateSource.Value;
 
-        public async System.Threading.Tasks.Task ConnectToOpenFileTrackerOnUIThreadAsync(IAsyncServiceProvider asyncServiceProvider)
+        internal void SubscribeExternalErrorDiagnosticUpdateSourceToSolutionBuildEvents()
+        {
+            ThreadHelper.ThrowIfNotOnUIThread();
+
+            if (_isExternalErrorDiagnosticUpdateSourceSubscribedToSolutionBuildEvents)
+            {
+                return;
+            }
+
+            // TODO: https://github.com/dotnet/roslyn/issues/36065
+            // UIContextImpl requires IVsMonitorSelection service:
+            if (ServiceProvider.GlobalProvider.GetService(typeof(IVsMonitorSelection)) == null)
+            {
+                return;
+            }
+
+            // This pattern ensures that we are called whenever the build starts/completes even if it is already in progress.
+            KnownUIContexts.SolutionBuildingContext.WhenActivated(() =>
+            {
+                KnownUIContexts.SolutionBuildingContext.UIContextChanged += (object _, UIContextChangedEventArgs e) =>
+                {
+                    if (e.Activated)
+                    {
+                        ExternalErrorDiagnosticUpdateSource.OnSolutionBuildStarted();
+                    }
+                    else
+                    {
+                        ExternalErrorDiagnosticUpdateSource.OnSolutionBuildCompleted();
+                    }
+                };
+
+                ExternalErrorDiagnosticUpdateSource.OnSolutionBuildStarted();
+            });
+
+            _isExternalErrorDiagnosticUpdateSourceSubscribedToSolutionBuildEvents = true;
+        }
+
+        public async Task InitializeUIAffinitizedServicesAsync(IAsyncServiceProvider asyncServiceProvider)
         {
             // Create services that are bound to the UI thread
             await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync();

src/VisualStudio/Core/Def/Implementation/TaskList/ProjectExternalErrorReporter.cs

@@ -19,7 +19,7 @@
 
 namespace Microsoft.VisualStudio.LanguageServices.Implementation.TaskList
 {
-    internal class ProjectExternalErrorReporter : IVsReportExternalErrors, IVsLanguageServiceBuildErrorReporter2
+    internal sealed class ProjectExternalErrorReporter : IVsReportExternalErrors, IVsLanguageServiceBuildErrorReporter2
     {
         internal static readonly IReadOnlyList<string> CustomTags = ImmutableArray.Create(WellKnownDiagnosticTags.Telemetry);
         internal static readonly IReadOnlyList<string> CompilerDiagnosticCustomTags = ImmutableArray.Create(WellKnownDiagnosticTags.Compiler, WellKnownDiagnosticTags.Telemetry);
@@ -33,6 +33,8 @@ internal class ProjectExternalErrorReporter : IVsReportExternalErrors, IVsLangua
         public ProjectExternalErrorReporter(ProjectId projectId, string errorCodePrefix, IServiceProvider serviceProvider)
             : this(projectId, errorCodePrefix, (VisualStudioWorkspaceImpl)serviceProvider.GetMefService<VisualStudioWorkspace>())
         {
+            ThreadHelper.ThrowIfNotOnUIThread();
+            _workspace.SubscribeExternalErrorDiagnosticUpdateSourceToSolutionBuildEvents();
         }
 
         public ProjectExternalErrorReporter(ProjectId projectId, string errorCodePrefix, VisualStudioWorkspaceImpl workspace)
@@ -44,28 +46,10 @@ public ProjectExternalErrorReporter(ProjectId projectId, string errorCodePrefix,
             _projectId = projectId;
             _errorCodePrefix = errorCodePrefix;
             _workspace = workspace;
-
-            KnownUIContexts.SolutionBuildingContext.WhenActivated(() =>
-            {
-                KnownUIContexts.SolutionBuildingContext.UIContextChanged += OnSolutionBuild;
-                DiagnosticProvider.OnSolutionBuildStarted();
-            });
         }
 
         private ExternalErrorDiagnosticUpdateSource DiagnosticProvider => _workspace.ExternalErrorDiagnosticUpdateSource;
 
-        private void OnSolutionBuild(object sender, UIContextChangedEventArgs e)
-        {
-            if (e.Activated)
-            {
-                DiagnosticProvider.OnSolutionBuildStarted();
-            }
-            else
-            {
-                DiagnosticProvider.OnSolutionBuildCompleted();
-            }
-        }
-
         private bool CanHandle(string errorId)
         {
             // make sure we have error id, otherwise, we simple don't support

src/VisualStudio/Core/Impl/ProjectSystem/CPS/CPSProject_IWorkspaceProjectContext.cs

@@ -12,6 +12,7 @@
 using Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel;
 using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList;
 using Microsoft.VisualStudio.LanguageServices.ProjectSystem;
+using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Shell.Interop;
 using Roslyn.Utilities;
 
@@ -57,6 +58,8 @@ public bool LastDesignTimeBuildSucceeded
 
         public CPSProject(VisualStudioProject visualStudioProject, VisualStudioWorkspaceImpl visualStudioWorkspace, IProjectCodeModelFactory projectCodeModelFactory, Guid projectGuid, string binOutputPath)
         {
+            ThreadHelper.ThrowIfNotOnUIThread();
+
             _visualStudioProject = visualStudioProject;
             _visualStudioWorkspace = visualStudioWorkspace;
 
@@ -73,6 +76,8 @@ public CPSProject(VisualStudioProject visualStudioProject, VisualStudioWorkspace
                 return (prefix != null) ? new ProjectExternalErrorReporter(visualStudioProject.Id, prefix, visualStudioWorkspace) : null;
             });
 
+            visualStudioWorkspace.SubscribeExternalErrorDiagnosticUpdateSourceToSolutionBuildEvents();
+
             _projectCodeModel = projectCodeModelFactory.CreateProjectCodeModel(visualStudioProject.Id, new CPSCodeModelInstanceFactory(this));
 
             // If we have a command line parser service for this language, also set up our ability to process options if they come in