From 112afcb744f3d8c0c703ffecfcd6fe92ab05964f Mon Sep 17 00:00:00 2001
From: Tom Meschter <tom.meschter@microsoft.com>
Date: Thu, 4 May 2017 14:44:43 -0700
Subject: [PATCH] Fix a small memory leak related to batching project loads

Fixes https://github.com/dotnet/project-system/issues/1918.

The language service implements `IVsSolutionLoadEvents.OnBeforeLoadProjectBatch` and `IVsSolutionLoadEvents.OnAfterLoadProjectBatch` to support asynchronous solution loads. While a batch is in progress we try to avoid pushing added projects to the workspace and instead store them in a list. When the batch is complete we may push all of those projects to the workspace at once.

The current implementation has a memory leak. Project may be added outside of a pair of `OnBeforeLoadProjectBatch`/`OnAfterLoadProjectBatch` calls, and it is possible for projects to be added without those methods ever being called. However, during a solution load adding a project always causes us to put it in the list of batched projects. Since we only clear the list on calls to `OnBeforeLoadProjectBatch` or `OnAfterLoadProjectBatch` the user could close the project or even the whole solution and we could still be holding on to one or more projects and all of their associated memory.

The fix here is to add a `bool` to track whether or not we're batching projects, and only put an added project in the list if we are.
---
 .../ProjectSystem/VisualStudioProjectTracker.cs       | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectTracker.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectTracker.cs
index 6cd13bf588c..71a8ab80602 100644
--- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectTracker.cs
+++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProjectTracker.cs
@@ -40,6 +40,13 @@ internal sealed partial class VisualStudioProjectTracker : ForegroundThreadAffin
 
         private readonly HostWorkspaceServices _workspaceServices;
 
+        /// <summary>
+        /// Set to true while we're batching project loads. That is, between
+        /// <see cref="IVsSolutionLoadEvents.OnBeforeLoadProjectBatch" /> and
+        /// <see cref="IVsSolutionLoadEvents.OnAfterLoadProjectBatch"/>.
+        /// </summary>
+        private bool _batchingProjectLoads = false;
+
         /// <summary>
         /// The list of projects loaded in this batch between <see cref="IVsSolutionLoadEvents.OnBeforeLoadProjectBatch" /> and
         /// <see cref="IVsSolutionLoadEvents.OnAfterLoadProjectBatch(bool)"/>.
@@ -282,7 +289,7 @@ internal void AddProject(AbstractProject project)
             {
                 StartPushingToWorkspaceAndNotifyOfOpenDocuments(SpecializedCollections.SingletonEnumerable(project));
             }
-            else
+            else if (_batchingProjectLoads)
             {
                 _projectsLoadedThisBatch.Add(project);
             }
@@ -916,6 +923,7 @@ internal void OnBeforeLoadProjectBatch(bool fIsBackgroundIdleBatch)
         {
             AssertIsForeground();
 
+            _batchingProjectLoads = true;
             _projectsLoadedThisBatch.Clear();
         }
 
@@ -930,6 +938,7 @@ internal void OnAfterLoadProjectBatch(bool fIsBackgroundIdleBatch)
                 StartPushingToWorkspaceAndNotifyOfOpenDocuments(_projectsLoadedThisBatch);
             }
 
+            _batchingProjectLoads = false;
             _projectsLoadedThisBatch.Clear();
         }
 
-- 
GitLab