[VSTest] Fix potential UI hang

On opening a solution containing a .NET Core test can sometimes
result in a UI hang. The IDE log shows a background thread and the
UI thread both stuck awaiting test discovery:

var discoveredTests = await VsTestDiscoveryAdapter.Instance.DiscoverTestsAsync (Project);

VsTestProjectTestSuite/<OnCreateTests>d__12.MoveNext
in MonoDevelop.UnitTesting.VsTest/VsTestProjectTestSuite.cs:95

Not been able to reproduce this problem directly by opening a
solution. However by modifying the code so a background thread
calls the VsTestDiscoveryAdapter.Instance just before the UI thread
tries to call it then the UI can hang. The problem here is that
the static Instance variable creates a new instance of the
VsTestDiscoveryAdapter. The VsTestDiscoveryAdapter's constructor
created a new progress monitor. The ProgressMonitorManager class
used is a GuiSyncObject so it will try to dispatch the call to
the UI thread. This blocks the background thread. Now the UI thread
attempts to access the VsTestDiscoveryAdapter.Instance and is blocked
resulting in a UI thread hang.

A small fix has been applied here to move the progress monitor
creation out of the constructor and into the DiscoveryTestsAsync
method. This prevents the UI thread hang.

Fixes VSTS #641134 - VS for mac freezes when opening solution

1 files changed, 26 insertions, 10 deletions

diff --git a/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.VsTest/VsTestDiscoveryAdapter.cs b/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.VsTest/VsTestDiscoveryAdapter.cs
index 0214dc7d9b..46c4ceecbc 100644
--- a/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.VsTest/VsTestDiscoveryAdapter.cs
+++ b/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.VsTest/VsTestDiscoveryAdapter.cs
@@ -43,17 +43,32 @@ namespace MonoDevelop.UnitTesting.VsTest
 	class VsTestDiscoveryAdapter : VsTestAdapter
 	{
 		ProgressMonitor monitor;
-		Pad pad;
-		public VsTestDiscoveryAdapter ()
+
+		/// <summary>
+		/// Creates the progress monitor if it does not already exist. This is done
+		/// on the UI thread explicitly. The ProgressMonitors is a GuiSyncObject so this
+		/// will be called on the UI thread implicitly otherwise. The progress monitor
+		/// should not be created when the static Instance is created since this can
+		/// result in a UI thread hang if Instance is called by both the UI thread and
+		/// a background thread at the same time.
+		/// </summary>
+		async Task CreateProgressMonitor ()
 		{
-			monitor = IdeApp.Workbench.ProgressMonitors.GetOutputProgressMonitor (
-				"TestDiscoveryConsole",
-				GettextCatalog.GetString ("Test Discovery Console"),
-				Stock.Console,
-				false,
-				true,
-				false);
-			pad = IdeApp.Workbench.ProgressMonitors.GetPadForMonitor (monitor);
+			if (monitor != null)
+				return;
+
+			await Runtime.RunInMainThread (() => {
+				if (monitor != null)
+					return;
+
+				monitor = IdeApp.Workbench.ProgressMonitors.GetOutputProgressMonitor (
+					"TestDiscoveryConsole",
+					GettextCatalog.GetString ("Test Discovery Console"),
+					Stock.Console,
+					false,
+					true,
+					false);
+			});
 		}
 
 		ConcurrentQueue<DiscoveryJob> discoveryQueue = new ConcurrentQueue<DiscoveryJob> ();
@@ -88,6 +103,7 @@ namespace MonoDevelop.UnitTesting.VsTest
 
 		public async Task<DiscoveredTests> DiscoverTestsAsync (Project project)
 		{
+			await CreateProgressMonitor ();
 			await Start ();
 			var job = new DiscoveryJob () { project = project };
 			discoveryQueue.Enqueue (job);