Merge pull request #250 from tonyhallett/fix-memory-leak

tonyhallett · web-flow · commit 96da8216f88b · 2022-03-29T16:05:44.000+01:00
fix #248
diff --git a/FineCodeCoverageTests/FCCEngine_Tests.cs b/FineCodeCoverageTests/FCCEngine_Tests.cs
@@ -30,11 +30,6 @@ public void SetUp()
             updatedMarginTags = false;
             mocker = new AutoMoqer();
             fccEngine = mocker.Create<FCCEngine>();
-            fccEngine.UpdateMarginTags += (UpdateMarginTagsEventArgs e) =>
-            {
-                updatedMarginTags = true;
-            };
-
         }
 
         [Test]
@@ -70,29 +65,20 @@ public void Should_Set_AppDataFolderPath_From_Initialized_AppDataFolder_Director
             fccEngine.Initialize(null, CancellationToken.None);
             Assert.AreEqual("some path", fccEngine.AppDataFolderPath);
         }
-    
+
         [Test]
-        public void Should_UpdateMarginTags_And_Set_Null_CoverageLines_When_ClearUI()
+        public void Should_Send_NewCoverageLinesMessage_With_Null_CoverageLines_When_ClearUI()
         {
-            fccEngine.CoverageLines = new List<CoverageLine>();
             fccEngine.ClearUI();
-            Assert.IsTrue(updatedMarginTags);
-            Assert.IsNull(fccEngine.CoverageLines);
+            mocker.Verify<IEventAggregator>(ea => ea.SendMessage(It.Is<NewCoverageLinesMessage>(msg => msg.CoverageLines == null), null));
         }
 
-        [Test]
-        public void Should_Begin_With_Null_CoverageLines()
-        {
-            Assert.IsNull(fccEngine.CoverageLines);
-        }
     }
 
     public class FCCEngine_ReloadCoverage_Tests
     {
         private AutoMoqer mocker;
         private FCCEngine fccEngine;
-        private List<UpdateMarginTagsEventArgs> updateMarginTagsEvents;
-        private List<List<CoverageLine>> updateMarginTagsCoverageLines;
 
         [SetUp]
         public void SetUp()
@@ -102,15 +88,6 @@ public void SetUp()
             mockDisposeAwareTaskRunner.Setup(runner => runner.RunAsync(It.IsAny<Func<Task>>())).Callback<Func<Task>>(async taskProvider => await taskProvider());
             fccEngine = mocker.Create<FCCEngine>();
 
-            updateMarginTagsEvents = new List<UpdateMarginTagsEventArgs>();
-            updateMarginTagsCoverageLines = new List<List<CoverageLine>>();
-
-            fccEngine.UpdateMarginTags += (UpdateMarginTagsEventArgs e) =>
-            {
-                updateMarginTagsEvents.Add(e);
-                updateMarginTagsCoverageLines.Add(fccEngine.CoverageLines);
-            };
-
             var mockedAppOptions = mocker.GetMock<IAppOptions>();
             mockedAppOptions.Setup(x => x.RunMsCodeCoverage).Returns(RunMsCodeCoverage.No);
             var mockAppOptionsProvider = mocker.GetMock<IAppOptionsProvider>();
@@ -356,11 +333,6 @@ private void VerifyLogsReloadCoverageStatus(ReloadCoverageStatus reloadCoverageS
             mocker.Verify<ILogger>(l => l.Log(fccEngine.GetLogReloadCoverageStatusMessage(reloadCoverageStatus)));
         }
 
-        private void VerifyClearUIEvents(int eventNumber)
-        {
-            Assert.Null(updateMarginTagsCoverageLines[eventNumber]);
-        }
-
         private async Task<(string reportGeneratedHtmlContent, List<CoverageLine> updatedCoverageLines)> RunToCompletion(bool noCoverageProjects)
         {
             var coverageProject = CreateSuitableProject().Object;
diff --git a/SharedProject/Core/FCCEngine.cs b/SharedProject/Core/FCCEngine.cs
@@ -17,6 +17,11 @@ namespace FineCodeCoverage.Engine
 {
     internal enum ReloadCoverageStatus { Start, Done, Cancelled, Error, Initializing };
 
+    internal class NewCoverageLinesMessage
+    {
+        public List<CoverageLine> CoverageLines { get; set; }
+    }
+
     internal class DisplayCoverageResultState
     {
         public CancellationTokenSource CancellationTokenSource { get; set; }
@@ -30,10 +35,7 @@ internal class FCCEngine : IFCCEngine,IDisposable
         internal const string initializationFailedMessagePrefix = "Initialization failed.  Please check the following error which may be resolved by reopening visual studio which will start the initialization process again.";
         private CancellationTokenSource cancellationTokenSource;
 
-        public event UpdateMarginTagsDelegate UpdateMarginTags;
-        
         public string AppDataFolderPath { get; private set; }
-        public List<CoverageLine> CoverageLines { get; internal set; }
         private bool IsVsShutdown => disposeAwareTaskRunner.DisposalToken.IsCancellationRequested;
 
         private readonly ICoverageUtilManager coverageUtilManager;
@@ -115,8 +117,7 @@ public void Initialize(IInitializeStatusProvider initializeStatusProvider, Cance
 
         public void ClearUI()
         {
-            CoverageLines = null;
-            UpdateMarginTags?.Invoke(new UpdateMarginTagsEventArgs());
+            ClearCoverageLines();
             ClearOutputWindow(true);
         }
 
@@ -139,8 +140,7 @@ public void StopCoverage()
         
         private CancellationTokenSource Reset()
         {
-            CoverageLines = null;
-            UpdateMarginTags?.Invoke(new UpdateMarginTagsEventArgs());
+            ClearCoverageLines();
 
             StopCoverage();
 
@@ -197,10 +197,19 @@ private void RaiseUpdateOutputWindow(string reportHtml)
             eventAggregator.SendMessage(new NewReportMessage { Report = reportHtml });
         }
 
+        private void ClearCoverageLines()
+        {
+            RaiseCoverageLines(null);
+        }
+
+        private void RaiseCoverageLines(List<CoverageLine> coverageLines)
+        {
+            eventAggregator.SendMessage(new NewCoverageLinesMessage { CoverageLines = coverageLines});
+        }
+
         private void UpdateUI(List<CoverageLine> coverageLines, string reportHtml)
         {
-            CoverageLines = coverageLines;
-            UpdateMarginTags?.Invoke(new UpdateMarginTagsEventArgs());
+            RaiseCoverageLines(coverageLines);
             if (reportHtml == null)
             {
                 reportHtml = reportGeneratorUtil.BlankReport(true);
diff --git a/SharedProject/Core/IFCCEngine.cs b/SharedProject/Core/IFCCEngine.cs
@@ -1,21 +1,18 @@
 ﻿using System;
 using System.Collections.Generic;
-using System.Threading.Tasks;
 using FineCodeCoverage.Engine.Model;
 using FineCodeCoverage.Impl;
 
 namespace FineCodeCoverage.Engine
 {
     internal interface IFCCEngine
     {
-        event UpdateMarginTagsDelegate UpdateMarginTags;
         string AppDataFolderPath { get; }
         void Initialize(IInitializeStatusProvider initializeStatusProvider, System.Threading.CancellationToken cancellationToken);
         void StopCoverage();
         void ReloadCoverage(Func<System.Threading.Tasks.Task<List<ICoverageProject>>> coverageRequestCallback);
         void RunAndProcessReport(string[] coberturaFiles,Action cleanUp = null);
         void ClearUI();
-        List<CoverageLine> CoverageLines { get; }
     }
 
 }
diff --git a/SharedProject/Impl/CoverageLineGlyphTagger.cs b/SharedProject/Impl/CoverageLineGlyphTagger.cs
@@ -5,24 +5,28 @@
 using Microsoft.VisualStudio.Text.Tagging;
 using FineCodeCoverage.Engine.Model;
 using System.Linq;
+using FineCodeCoverage.Core.Utilities;
 
 namespace FineCodeCoverage.Impl
 {
-	internal class CoverageLineGlyphTagger : ITagger<CoverageLineGlyphTag>
+	internal class CoverageLineGlyphTagger : ITagger<CoverageLineGlyphTag>, IListener<NewCoverageLinesMessage>
 	{
 		private readonly ITextBuffer _textBuffer;
-        private readonly IFCCEngine fccEngine;
+		private List<CoverageLine> coverageLines;
 
 		public event EventHandler<SnapshotSpanEventArgs> TagsChanged;
 
-		public CoverageLineGlyphTagger(ITextBuffer textBuffer, IFCCEngine fccEngine)
+		public CoverageLineGlyphTagger(ITextBuffer textBuffer, List<CoverageLine> lastCoverageLines)
 		{
 			_textBuffer = textBuffer;
-            this.fccEngine = fccEngine;
-            fccEngine.UpdateMarginTags += FCCEngine_UpdateMarginTags;
+			coverageLines = lastCoverageLines;
+			if (lastCoverageLines != null)
+            {
+				RaiseTagsChanged();
+            }
 		}
 
-		private void FCCEngine_UpdateMarginTags(UpdateMarginTagsEventArgs e)
+		private void RaiseTagsChanged()
 		{
 			var span = new SnapshotSpan(_textBuffer.CurrentSnapshot, 0, _textBuffer.CurrentSnapshot.Length);
 			var spanEventArgs = new SnapshotSpanEventArgs(span);
@@ -33,7 +37,7 @@ IEnumerable<ITagSpan<CoverageLineGlyphTag>> ITagger<CoverageLineGlyphTag>.GetTag
 		{
 			var result = new List<ITagSpan<CoverageLineGlyphTag>>();
 
-			if (spans == null || fccEngine.CoverageLines == null)
+			if (spans == null || coverageLines == null)
 			{
 				return result;
 			}
@@ -47,11 +51,11 @@ IEnumerable<ITagSpan<CoverageLineGlyphTag>> ITagger<CoverageLineGlyphTag>.GetTag
 
 				var startLineNumber = span.Start.GetContainingLine().LineNumber + 1;
 				var endLineNumber = span.End.GetContainingLine().LineNumber + 1;
-				var coverageLines = GetLines(document.FilePath, startLineNumber, endLineNumber);
+				var applicableCoverageLines = GetApplicableLines(document.FilePath, startLineNumber, endLineNumber);
 
-				foreach (var coverageLine in coverageLines)
+				foreach (var applicableCoverageLine in applicableCoverageLines)
 				{
-					var tag = new CoverageLineGlyphTag(coverageLine);
+					var tag = new CoverageLineGlyphTag(applicableCoverageLine);
 					var tagSpan = new TagSpan<CoverageLineGlyphTag>(span, tag);
 					result.Add(tagSpan);
 				}
@@ -60,13 +64,19 @@ IEnumerable<ITagSpan<CoverageLineGlyphTag>> ITagger<CoverageLineGlyphTag>.GetTag
 			return result;
 		}
 
-		private IEnumerable<CoverageLine> GetLines(string filePath, int startLineNumber, int endLineNumber)
+		private IEnumerable<CoverageLine> GetApplicableLines(string filePath, int startLineNumber, int endLineNumber)
 		{
-			return fccEngine.CoverageLines
+			return coverageLines
 			.AsParallel()
 			.Where(x => x.Class.Filename.Equals(filePath, StringComparison.OrdinalIgnoreCase))
 			.Where(x => x.Line.Number >= startLineNumber && x.Line.Number <= endLineNumber)
 			.ToArray();
 		}
-	}
+
+        public void Handle(NewCoverageLinesMessage message)
+        {
+            coverageLines = message.CoverageLines;
+			RaiseTagsChanged();
+        }
+    }
 }
diff --git a/SharedProject/Impl/CoverageLineGlyphTaggerProvider.cs b/SharedProject/Impl/CoverageLineGlyphTaggerProvider.cs
@@ -4,42 +4,48 @@
 using Microsoft.VisualStudio.Text.Tagging;
 using FineCodeCoverage.Engine;
 using Microsoft.VisualStudio.Shell;
+using FineCodeCoverage.Core.Utilities;
+using System.Collections.Generic;
+using FineCodeCoverage.Engine.Model;
 
 namespace FineCodeCoverage.Impl
 {
 	[ContentType("code")]
 	[TagType(typeof(CoverageLineGlyphTag))]
 	[Name(Vsix.TaggerProviderName)]
 	[Export(typeof(ITaggerProvider))]
-	internal class CoverageLineGlyphTaggerProvider : ITaggerProvider
+	internal class CoverageLineGlyphTaggerProvider : ITaggerProvider, IListener<NewCoverageLinesMessage>
 	{
-        private readonly IFCCEngine fccEngine;
+        private readonly IEventAggregator eventAggregator;
         private readonly ICoverageColoursProvider coverageColoursProvider;
+        private List<CoverageLine> lastCoverageLines;
 
         [ImportingConstructor]
 		public CoverageLineGlyphTaggerProvider(
-            IFCCEngine fccEngine, 
+            IEventAggregator eventAggregator, 
             ICoverageColoursProvider coverageColoursProvider)
         {
-            this.fccEngine = fccEngine;
-            fccEngine.UpdateMarginTags += FccEngine_UpdateMarginTags;
+            eventAggregator.AddListener(this);
+            this.eventAggregator = eventAggregator;
             this.coverageColoursProvider = coverageColoursProvider;
         }
 
-        private void FccEngine_UpdateMarginTags(UpdateMarginTagsEventArgs e)
+        public ITagger<T> CreateTagger<T>(ITextBuffer textBuffer) where T : ITag
+		{
+			var coverageLineGlyphTagger =  new CoverageLineGlyphTagger(textBuffer, lastCoverageLines);
+            eventAggregator.AddListener(coverageLineGlyphTagger, false);
+            return coverageLineGlyphTagger as ITagger<T>;
+		}
+
+        public void Handle(NewCoverageLinesMessage message)
         {
+            lastCoverageLines = message.CoverageLines;
 #pragma warning disable VSTHRD102 // Implement internal logic asynchronously
-			ThreadHelper.JoinableTaskFactory.Run(async () =>
+            ThreadHelper.JoinableTaskFactory.Run(async () =>
 #pragma warning restore VSTHRD102 // Implement internal logic asynchronously
-			{
-				await coverageColoursProvider.PrepareAsync();
-			});
-			
+            {
+                await coverageColoursProvider.PrepareAsync();
+            });
         }
-
-        public ITagger<T> CreateTagger<T>(ITextBuffer textBuffer) where T : ITag
-		{
-			return new CoverageLineGlyphTagger(textBuffer, fccEngine) as ITagger<T>;
-		}
-	}
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -1,21 +1,18 @@`
`1`	`1`	`using System;`
`2`	`2`	`using System.Collections.Generic;`
`3`		`-using System.Threading.Tasks;`
`4`	`3`	`using FineCodeCoverage.Engine.Model;`
`5`	`4`	`using FineCodeCoverage.Impl;`
`6`	`5`
`7`	`6`	`namespace FineCodeCoverage.Engine`
`8`	`7`	`{`
`9`	`8`	`internal interface IFCCEngine`
`10`	`9`	`{`
`11`		`- event UpdateMarginTagsDelegate UpdateMarginTags;`
`12`	`10`	`string AppDataFolderPath { get; }`
`13`	`11`	`void Initialize(IInitializeStatusProvider initializeStatusProvider, System.Threading.CancellationToken cancellationToken);`
`14`	`12`	`void StopCoverage();`
`15`	`13`	`void ReloadCoverage(Func<System.Threading.Tasks.Task<List<ICoverageProject>>> coverageRequestCallback);`
`16`	`14`	`void RunAndProcessReport(string[] coberturaFiles,Action cleanUp = null);`
`17`	`15`	`void ClearUI();`
`18`		`- List<CoverageLine> CoverageLines { get; }`
`19`	`16`	`}`
`20`	`17`
`21`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,24 +5,28 @@`
`5`	`5`	`using Microsoft.VisualStudio.Text.Tagging;`
`6`	`6`	`using FineCodeCoverage.Engine.Model;`
`7`	`7`	`using System.Linq;`
	`8`	`+using FineCodeCoverage.Core.Utilities;`
`8`	`9`
`9`	`10`	`namespace FineCodeCoverage.Impl`
`10`	`11`	`{`
`11`		`- internal class CoverageLineGlyphTagger : ITagger<CoverageLineGlyphTag>`
	`12`	`+ internal class CoverageLineGlyphTagger : ITagger<CoverageLineGlyphTag>, IListener<NewCoverageLinesMessage>`
`12`	`13`	`{`
`13`	`14`	`private readonly ITextBuffer _textBuffer;`
`14`		`- private readonly IFCCEngine fccEngine;`
	`15`	`+ private List<CoverageLine> coverageLines;`
`15`	`16`
`16`	`17`	`public event EventHandler<SnapshotSpanEventArgs> TagsChanged;`
`17`	`18`
`18`		`- public CoverageLineGlyphTagger(ITextBuffer textBuffer, IFCCEngine fccEngine)`
	`19`	`+ public CoverageLineGlyphTagger(ITextBuffer textBuffer, List<CoverageLine> lastCoverageLines)`
`19`	`20`	`{`
`20`	`21`	`_textBuffer = textBuffer;`
`21`		`- this.fccEngine = fccEngine;`
`22`		`- fccEngine.UpdateMarginTags += FCCEngine_UpdateMarginTags;`
	`22`	`+ coverageLines = lastCoverageLines;`
	`23`	`+ if (lastCoverageLines != null)`
	`24`	`+ {`
	`25`	`+ RaiseTagsChanged();`
	`26`	`+ }`
`23`	`27`	`}`
`24`	`28`
`25`		`- private void FCCEngine_UpdateMarginTags(UpdateMarginTagsEventArgs e)`
	`29`	`+ private void RaiseTagsChanged()`
`26`	`30`	`{`
`27`	`31`	`var span = new SnapshotSpan(_textBuffer.CurrentSnapshot, 0, _textBuffer.CurrentSnapshot.Length);`
`28`	`32`	`var spanEventArgs = new SnapshotSpanEventArgs(span);`
`@@ -33,7 +37,7 @@ IEnumerable<ITagSpan<CoverageLineGlyphTag>> ITagger<CoverageLineGlyphTag>.GetTag`
`33`	`37`	`{`
`34`	`38`	`var result = new List<ITagSpan<CoverageLineGlyphTag>>();`
`35`	`39`
`36`		`- if (spans == null \|\| fccEngine.CoverageLines == null)`
	`40`	`+ if (spans == null \|\| coverageLines == null)`
`37`	`41`	`{`
`38`	`42`	`return result;`
`39`	`43`	`}`
`@@ -47,11 +51,11 @@ IEnumerable<ITagSpan<CoverageLineGlyphTag>> ITagger<CoverageLineGlyphTag>.GetTag`
`47`	`51`
`48`	`52`	`var startLineNumber = span.Start.GetContainingLine().LineNumber + 1;`
`49`	`53`	`var endLineNumber = span.End.GetContainingLine().LineNumber + 1;`
`50`		`- var coverageLines = GetLines(document.FilePath, startLineNumber, endLineNumber);`
	`54`	`+ var applicableCoverageLines = GetApplicableLines(document.FilePath, startLineNumber, endLineNumber);`
`51`	`55`
`52`		`- foreach (var coverageLine in coverageLines)`
	`56`	`+ foreach (var applicableCoverageLine in applicableCoverageLines)`
`53`	`57`	`{`
`54`		`- var tag = new CoverageLineGlyphTag(coverageLine);`
	`58`	`+ var tag = new CoverageLineGlyphTag(applicableCoverageLine);`
`55`	`59`	`var tagSpan = new TagSpan<CoverageLineGlyphTag>(span, tag);`
`56`	`60`	`result.Add(tagSpan);`
`57`	`61`	`}`
`@@ -60,13 +64,19 @@ IEnumerable<ITagSpan<CoverageLineGlyphTag>> ITagger<CoverageLineGlyphTag>.GetTag`
`60`	`64`	`return result;`
`61`	`65`	`}`
`62`	`66`
`63`		`- private IEnumerable<CoverageLine> GetLines(string filePath, int startLineNumber, int endLineNumber)`
	`67`	`+ private IEnumerable<CoverageLine> GetApplicableLines(string filePath, int startLineNumber, int endLineNumber)`
`64`	`68`	`{`
`65`		`- return fccEngine.CoverageLines`
	`69`	`+ return coverageLines`
`66`	`70`	`.AsParallel()`
`67`	`71`	`.Where(x => x.Class.Filename.Equals(filePath, StringComparison.OrdinalIgnoreCase))`
`68`	`72`	`.Where(x => x.Line.Number >= startLineNumber && x.Line.Number <= endLineNumber)`
`69`	`73`	`.ToArray();`
`70`	`74`	`}`
`71`		`- }`
	`75`	`+`
	`76`	`+ public void Handle(NewCoverageLinesMessage message)`
	`77`	`+ {`
	`78`	`+ coverageLines = message.CoverageLines;`
	`79`	`+ RaiseTagsChanged();`
	`80`	`+ }`
	`81`	`+ }`
`72`	`82`	`}`