cherry pick CLI deadlock fix to 2.19.5 (#15030)

master-15 has some flaky tests, but they were fixed in master so I will merge this.

src/DynamoCoreWpf/Views/SplashScreen/SplashScreen.xaml.cs

@@ -511,6 +511,7 @@ protected override void OnClosed(EventArgs e)
 
             DynamoModel.RequestUpdateLoadBarStatus -= DynamoModel_RequestUpdateLoadBarStatus;
             DynamoModel.LanguageDetected -= DynamoModel_LanguageDetected;
+            StaticSplashScreenReady -= OnStaticScreenReady;
             webView.Dispose();
             webView = null;
 

src/DynamoUtilities/CLIWrapper.cs

@@ -96,8 +96,9 @@ protected static string GetToolPath(string relativePath)
         /// Read data from CLI tool
         /// </summary>
         /// <param name="timeoutms">will return empty string if we don't finish reading all data in the timeout provided in milliseconds.</param>
+        /// <param name="mockReadLine"> if this delegate is non null, it will be used instead of communicating with std out of the process. Used for testing only.</param>
         /// <returns></returns>
-        protected virtual async Task<string> GetData(int timeoutms)
+        protected virtual string GetData(int timeoutms, Func<string> mockReadLine = null)
         {
             var readStdOutTask = Task.Run(() =>
             {
@@ -114,7 +115,17 @@ protected virtual async Task<string> GetData(int timeoutms)
                     {
                         try
                         {
-                            var line = process.StandardOutput.ReadLine();
+                            string line = null;
+                            if(mockReadLine != null)
+                            {
+                                line = mockReadLine.Invoke();
+                            }
+                            else
+                            {
+                                line = process.StandardOutput.ReadLine();
+                            }
+
+
                             MessageLogged?.Invoke(line);
                             if (line == null || line == startofDataToken)
                             {
@@ -145,7 +156,7 @@ protected virtual async Task<string> GetData(int timeoutms)
                     return writer.ToString();
                 }
             });
-            var completedTask = await Task.WhenAny(readStdOutTask, Task.Delay(TimeSpan.FromMilliseconds(timeoutms)));
+            var completedTask = Task.WhenAny(readStdOutTask, Task.Delay(TimeSpan.FromMilliseconds(timeoutms))).Result;
             //if the completed task was our read std out task, then return the data
             //else we timed out, so return an empty string.
             return completedTask == readStdOutTask ? readStdOutTask.Result : string.Empty;

src/DynamoUtilities/DynamoFeatureFlagsManager.cs

@@ -64,7 +64,7 @@ internal void CacheAllFlags()
         {
 
             //wait for response
-            var dataFromCLI = GetData(featureFlagTimeoutMs).Result;    
+            var dataFromCLI = GetData(featureFlagTimeoutMs);    
             //convert from json string to dictionary.
             try
             {

src/DynamoUtilities/Md2Html.cs

@@ -73,9 +73,7 @@ internal string ParseMd2Html(string mdString, string mdPath)
                 return GetCantCommunicateErrorMessage();
             }
 
-            var output = GetData(processCommunicationTimeoutms);
-
-            return output.Result;
+            return GetData(processCommunicationTimeoutms);
         }
 
         /// <summary>
@@ -102,9 +100,7 @@ internal string SanitizeHtml(string content)
                 return GetCantCommunicateErrorMessage();
             }
 
-            var output = GetData(processCommunicationTimeoutms);
-
-           return output.Result;
+            return GetData(processCommunicationTimeoutms);
         }
 
         /// <summary>

test/Libraries/DynamoUtilitiesTests/CLIWrapperTests.cs

@@ -3,6 +3,7 @@
 using System.IO;
 using System.Linq;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 using NUnit.Framework;
 
@@ -39,7 +40,46 @@ internal string GetData()
                 {   System.Threading.Thread.Sleep(100);
                     process.Kill();
                 });
-                return GetData(2000).Result;
+                return GetData(2000);
+            }
+
+
+        }
+        /// <summary>
+        /// this test class waits before reading from the console, so GetData is slow.
+        /// </summary>
+        private class SlowCLIWrapper : HangingCLIWrapper
+        {
+            internal new string GetData()
+            {
+                return GetData(2000, () => { Thread.Sleep(4000);return ""; });
+            }
+        }
+
+        /// <summary>
+        /// this test class should get mock data and should not time out.
+        /// </summary>
+        private class MockCLIWraper : HangingCLIWrapper
+        {
+            int count = 0;
+            internal new string GetData()
+            {
+                return GetData(2000, () => {
+                    count++;
+
+                    switch (count)
+                    {
+                        case 1:
+                            return startofDataToken;
+                        case 2:
+                            return "some data";
+                        case 3:
+                            return endOfDataToken;
+                        default:
+                            return string.Empty;
+                    }
+
+                });
             }
         }
 
@@ -54,5 +94,27 @@ public void CLIWrapperDoesNotHangIfProcessDoesNotWriteToStdOut()
             Assert.GreaterOrEqual(sw.ElapsedMilliseconds,2000);
 
         }
+        [Test]
+        public void CLIWrapperTimesOutIfGetDataIsSlow()
+        {
+            var sw = new System.Diagnostics.Stopwatch();
+            sw.Start();
+            var wrapper = new SlowCLIWrapper();
+            Assert.AreEqual(string.Empty, wrapper.GetData());
+            sw.Stop();
+            Assert.GreaterOrEqual(sw.ElapsedMilliseconds, 2000);
+
+        }
+        [Test]
+        public void CLIGetsDataIfDoesNotTimeout()
+        {
+            var sw = new System.Diagnostics.Stopwatch();
+            sw.Start();
+            var wrapper = new MockCLIWraper();
+            Assert.AreEqual("some data", wrapper.GetData().TrimEnd());
+            sw.Stop();
+            Assert.LessOrEqual(sw.ElapsedMilliseconds, 2000);
+
+        }
     }
 }