diff options
author | Lluis Sanchez <lluis@xamarin.com> | 2019-07-03 20:14:25 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-07-03 20:14:25 +0300 |
commit | 5b135742da1ecd7a7f9dbe0e0faf9b8b160c93e6 (patch) | |
tree | 1649899d298c1ae1f90284b503c3200cb44f5ed5 /main | |
parent | 8d39943815841281dba20161dd97333c0abc71b1 (diff) | |
parent | 5622095c5cff00daabe9de2a73bdba336bf6132e (diff) |
Merge pull request #8118 from mono/backport-pr-8103-to-release-8.2
[release-8.2] [Core] Fix null ref in DotNetProject.OnGetReferencedAssemblies
Diffstat (limited to 'main')
-rw-r--r-- | main/src/core/MonoDevelop.Core/MonoDevelop.Projects/WorkspaceObject.cs | 46 | ||||
-rw-r--r-- | main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/ProjectTests.cs | 82 |
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); + } + } } } |