[Core] Fix build engine not being removed

The change make the call to RemoteBuildEngine.UnloadProject meant
that the engine in the RemoteProjectBuilder was being set to null
and the RemoteBuildEngineManager.ReleaseProjectBuilder was being
called with this null engine. Reworked the code so that if the
builder is being disposed then the call to ReleaseProjectBuilder
is done after the UnloadProject.

Also modified a builder test so it waits a while for the engine
count to be reduced since that is now done asynchronously.

2 files changed, 24 insertions, 4 deletions

diff --git a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.MSBuild/RemoteProjectBuilder.cs b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.MSBuild/RemoteProjectBuilder.cs
index aa73ccf88f..5f5d3e6c5d 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.MSBuild/RemoteProjectBuilder.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.MSBuild/RemoteProjectBuilder.cs
@@ -243,8 +243,9 @@ namespace MonoDevelop.Projects.MSBuild
 			lock (usageLock) {
 				if (--references == 0) {
 					if (shuttingDown)
-						Dispose ();
-					RemoteBuildEngineManager.ReleaseProjectBuilder (engine).Ignore ();
+						Dispose (releaseProjectBuilder: true);
+					else
+						RemoteBuildEngineManager.ReleaseProjectBuilder (engine).Ignore ();
 				}
 			}
 		}
@@ -260,7 +261,7 @@ namespace MonoDevelop.Projects.MSBuild
 			}
 		}
 
-		async void Dispose ()
+		async void Dispose (bool releaseProjectBuilder = false)
 		{
 			if (!MSBuildProjectService.ShutDown && engine != null) {
 				var currentEngine = engine;
@@ -271,6 +272,9 @@ namespace MonoDevelop.Projects.MSBuild
 					} catch {
 						// Ignore
 					}
+
+					if (releaseProjectBuilder)
+						await RemoteBuildEngineManager.ReleaseProjectBuilder (currentEngine);
 				}).Ignore ();
 				GC.SuppressFinalize (this);
 				engine = null;
diff --git a/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/BuilderManagerTests.cs b/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/BuilderManagerTests.cs
index d8f759ade8..8078405ef5 100644
--- a/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/BuilderManagerTests.cs
+++ b/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/BuilderManagerTests.cs
@@ -621,14 +621,30 @@ namespace MonoDevelop.Projects
 
 					// The builder that was running the build and was shutdown should be immediately stopped after build finishes
 					Assert.AreEqual (0, RemoteBuildEngineManager.ActiveEnginesCount);
-					Assert.AreEqual (0, RemoteBuildEngineManager.EnginesCount);
 
+					await AssertWithTimeout (
+						10000,
+						() => 0 == RemoteBuildEngineManager.EnginesCount,
+						() => "Expecting 0 RemoteBuildEngineManager.EnginesCount but was " + RemoteBuildEngineManager.EnginesCount);
 				}
 			} finally {
 				RemoteBuildEngineManager.EngineDisposalDelay = currentDelay;
 			}
 		}
 
+		async Task AssertWithTimeout (int timeout, Func<bool> checkTest, Func<string> getFailureMessage)
+		{
+			int checkInterval = 100;
+			int timeWaited = 0;
+			while (!checkTest ()) {
+				await Task.Delay (checkInterval);
+				timeWaited += checkInterval;
+				if (timeWaited >= timeout) {
+					Assert.Fail (getFailureMessage ());
+				}
+			}
+		}
+
 		[Test]
 		public async Task ParallelBuilds ()
 		{