[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 dda9d7f6ef..62ff9ceaa1 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.MSBuild/RemoteProjectBuilder.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.MSBuild/RemoteProjectBuilder.cs
@@ -241,8 +241,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 ();
 				}
 			}
 		}
@@ -258,7 +259,7 @@ namespace MonoDevelop.Projects.MSBuild
 			}
 		}
 
-		async void Dispose ()
+		async void Dispose (bool releaseProjectBuilder = false)
 		{
 			if (!MSBuildProjectService.ShutDown && engine != null) {
 				var currentEngine = engine;
@@ -269,6 +270,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 587dc1d870..6e54d1cf91 100644
--- a/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/BuilderManagerTests.cs
+++ b/main/tests/MonoDevelop.Core.Tests/MonoDevelop.Projects/BuilderManagerTests.cs
@@ -620,14 +620,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 ()
 		{