Merge pull request #8118 from mono/backport-pr-8103-to-release-8.2

[release-8.2] [Core] Fix null ref in DotNetProject.OnGetReferencedAssemblies

2 files changed, 120 insertions, 8 deletions

diff --git a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/WorkspaceObject.cs b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/WorkspaceObject.cs
index 459a5b2359..816653dd0c 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/WorkspaceObject.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/WorkspaceObject.cs
@@ -98,15 +98,30 @@ namespace MonoDevelop.Projects
 		/// <typeparam name="T">Task return type</typeparam>
 		public Task<T> BindTask<T> (Func<CancellationToken, Task<T>> f)
 		{
-			var t = f (disposeCancellation.Token);
+			var taskCompletionSource = new TaskCompletionSource<bool> ();
 			lock (activeTasks) {
+				// Need to register a task before the Func is called otherwise the Dispose could be called whilst the
+				// Func's Task is being run and before the Func's task is registered.
 				if (disposeCancellation.IsCancellationRequested)
 					return Task.FromCanceled<T> (disposeCancellation.Token);
-				activeTasks.Add (t);
+				activeTasks.Add (taskCompletionSource.Task);
+			}
+			Task<T> t;
+			try {
+				t = f (disposeCancellation.Token);
+			} catch (Exception ex) {
+				// Do not prevent Project.OnDispose being called by leaving the task on the activeTasks list.
+				lock (activeTasks) {
+					taskCompletionSource.SetResult (true);
+					activeTasks.Remove (taskCompletionSource.Task);
+				}
+				return Task.FromException<T> (ex);
 			}
 			t.ContinueWith (tr => {
-				lock (activeTasks)
-					activeTasks.Remove (t);
+				lock (activeTasks) {
+					taskCompletionSource.SetResult (true);
+					activeTasks.Remove (taskCompletionSource.Task);
+				}
 			});
 			return t;
 		}
@@ -120,15 +135,30 @@ namespace MonoDevelop.Projects
 		/// to be tracked. The provided CancellationToken will be signalled when the object is disposed.</param>
 		public Task BindTask (Func<CancellationToken, Task> f)
 		{
-			var t = f (disposeCancellation.Token);
+			var taskCompletionSource = new TaskCompletionSource<bool> ();
 			lock (activeTasks) {
+				// Need to register a task before the Func is called otherwise the Dispose could be called whilst the
+				// Func's Task is being run and before the Func's task is registered.
 				if (disposeCancellation.IsCancellationRequested)
 					return Task.FromCanceled (disposeCancellation.Token);
-				activeTasks.Add (t);
+				activeTasks.Add (taskCompletionSource.Task);
+			}
+			Task t;
+			try {
+				t = f (disposeCancellation.Token);
+			} catch (Exception ex) {
+				// Do not prevent Project.OnDispose being called by leaving the task on the activeTasks list.
+				lock (activeTasks) {
+					taskCompletionSource.SetResult (true);
+					activeTasks.Remove (taskCompletionSource.Task);
+				}
+				return Task.FromException (ex);
 			}
 			t.ContinueWith (tr => {
-				lock (activeTasks)
-					activeTasks.Remove (t);
+				lock (activeTasks) {
+					taskCompletionSource.SetResult (true);
+					activeTasks.Remove (taskCompletionSource.Task);
+				}
 			});
 			return t;
 		}
diff --git a/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/ProjectTests.cs b/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/ProjectTests.cs
index 0d5daeedcd..e44b98f5c6 100644
--- a/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/ProjectTests.cs
+++ b/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/ProjectTests.cs
@@ -33,6 +33,7 @@ using NUnit.Framework;
 using UnitTests;
 using MonoDevelop.Core;
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using MonoDevelop.Projects.Policies;
 using System.Xml;
@@ -1153,5 +1154,86 @@ namespace MonoDevelop.Projects
 				ModifiedEventArgs.Add (args);
 			}
 		}
+
+		/// <summary>
+		/// Tests that the project will be disposed even if something fails when the Func passed to
+		/// BindTask throws an exception. We want to ensure that the active task is cleared if there is
+		/// an error to avoid the project waiting forever for this task to finish on disposing.
+		/// </summary>
+		[Test]
+		public async Task BindTask_FuncThrowsException_ProjectIsDisposed_ProjectDoesNotWaitForTask ()
+		{
+			string solFile = Util.GetSampleProject ("console-project", "ConsoleProject.sln");
+			using (Solution sol = (Solution)await Services.ProjectService.ReadWorkspaceItem (Util.GetMonitor (), solFile)) {
+				var p = (DotNetProject)sol.Items [0];
+
+				Task task = null;
+				try {
+					task = p.BindTask (arg => throw new ApplicationException ("Test"));
+					await task;
+					Assert.Fail ("Should not reach this line");
+				} catch (ApplicationException) {
+				}
+
+				Assert.IsNotNull (task);
+
+				Task<string> task2 = null;
+				try {
+					task2 = p.BindTask<string> (arg => throw new ApplicationException ("Test 2"));
+					await task2;
+					Assert.Fail ("Should not reach this line - BindTask<T>");
+				} catch (ApplicationException) {
+				}
+
+				Assert.IsNotNull (task2);
+
+				p.Dispose ();
+
+				const int timeout = 5000; // ms
+				int howLong = 0;
+				const int interval = 200; // ms
+
+				while (p.MSBuildProject != null) {
+					if (howLong >= timeout)
+						Assert.Fail ("Timed out waiting for project to be disposed");
+
+					await Task.Delay (interval);
+					howLong += interval;
+				}
+			}
+		}
+
+		[Test]
+		public async Task GetReferences_ProjectDisposed_BeforeTaskIsBound_DoesNotThrowNullReferenceException ()
+		{
+			var fn = new CustomItemNode<TestGetReferencesProjectExtension> ();
+			WorkspaceObject.RegisterCustomExtension (fn);
+
+			try {
+				string solFile = Util.GetSampleProject ("console-project", "ConsoleProject.sln");
+				using (Solution sol = (Solution)await Services.ProjectService.ReadWorkspaceItem (Util.GetMonitor (), solFile)) {
+					var p = (DotNetProject)sol.Items [0];
+
+					// The TestGetReferencesProjectExtension will call Project.Dispose during the GetReferences call.
+					// Previously the GetReferences would throw a TaskCanceledException and Project.OnGetReferences would
+					// throw a unhandled NullReferenceException since the project was disposed which results in MSBuildProject
+					// being set to null.
+					var refs = await p.GetReferences (ConfigurationSelector.Default, CancellationToken.None);
+					Assert.IsTrue (refs.Any (r => r.FilePath.FileName == "System.Xml.dll"));
+				}
+			} finally {
+				WorkspaceObject.UnregisterCustomExtension (fn);
+			}
+		}
+
+		class TestGetReferencesProjectExtension : DotNetProjectExtension
+		{
+			protected internal override Task<List<AssemblyReference>> OnGetReferences (ConfigurationSelector configuration, CancellationToken token)
+			{
+				// Simulate the project being disposed whilst BindTask is called.
+				Project.Dispose ();
+				return base.OnGetReferences (configuration, token);
+			}
+		}
 	}
 }