Respond to PR feedback

build/scripts/cibuild.ps1

@@ -55,6 +55,13 @@ function Terminate-BuildProcesses() {
 # Ensure that procdump is available on the machine.  Returns the path to the directory that contains 
 # the procdump binaries (both 32 and 64 bit)
 function Ensure-ProcDump() {
+
+    # Jenkins images default to having procdump installed in the root.  Use that if available to avoid
+    # an unnecessary download.
+    if (Test-Path "c:\SysInternals\procdump.exe") {
+        return "c:\SysInternals";
+    }    
+
     $toolsDir = Join-Path $binariesDir "Tools"
     $outDir = Join-Path $toolsDir "ProcDump"
     $filePath = Join-Path $outDir "procdump.exe"

src/Tools/Source/RunTests/Cache/CachingTestExecutor.cs

@@ -124,7 +124,7 @@ private async Task CacheTestResult(ContentFile contentFile, TestResult testResul
             }
             catch (Exception ex)
             {
-                Logger.Log($"Failed to create cached {ex}");
+                Logger.Log("Failed to create cached", ex);
             }
         }
     }

src/Tools/Source/RunTests/Cache/LocalDataStorage.cs

@@ -80,7 +80,7 @@ public bool TryGetCachedTestResult(string checksum, out CachedTestResult testRes
             catch (Exception e)
             {
                 // Okay for exception to occur here on I/O
-                Logger.Log($"Failed to read cache {checksum} {e.Message}");
+                Logger.Log($"Failed to read cache {checksum}", e);
             }
 
             return false;
@@ -107,7 +107,7 @@ public Task AddCachedTestResult(AssemblyInfo assemblyInfo, ContentFile contentFi
             catch (Exception e)
             {
                 // I/O errors are expected and okay here.
-                Logger.Log($"Failed to log {checksum} {e.Message}");
+                Logger.Log($"Failed to log {checksum}", e);
                 FileUtil.DeleteDirectory(storagePath);
             }
 
@@ -159,7 +159,7 @@ private void CleanupStorage()
             }
             catch (Exception ex)
             {
-                Logger.Log($"Unable to cleanup storage {ex.Message}");
+                Logger.Log("Unable to cleanup storage", ex);
             }
         }
     }

src/Tools/Source/RunTests/Logger.cs

@@ -26,6 +26,25 @@ internal static void LogError(Exception ex, string line)
             }
         }
 
+        internal static void Log(string message, Exception ex)
+        {
+            lock (s_lines)
+            {
+                s_lines.Add(message);
+                s_lines.Add(ex.Message);
+                s_lines.Add(ex.StackTrace);
+            }
+        }
+
+        internal static void Log(Exception ex)
+        {
+            lock (s_lines)
+            {
+                s_lines.Add(ex.Message);
+                s_lines.Add(ex.StackTrace);
+            }
+        }
+
         internal static void Log(string line)
         {
             lock (s_lines)

src/Tools/Source/RunTests/ProcessRunner.cs

@@ -36,8 +36,8 @@ public static void OpenFile(string file)
         public static Task<ProcessOutput> RunProcessAsync(
             string executable,
             string arguments,
-            bool lowPriority,
             CancellationToken cancellationToken,
+            bool lowPriority = false,
             string workingDirectory = null,
             bool captureOutput = false,
             bool displayWindow = true,

src/Tools/Source/RunTests/ProcessUtil.cs

@@ -78,25 +78,5 @@ internal static List<Process> GetProcessTree(Process process)
 
             return list;
         }
-
-        internal static bool Is64Bit(Process process)
-        {
-            if (Environment.GetEnvironmentVariable("PROCESSOR_ARCHITECTURE") == "x86")
-            {
-                return false;
-            }
-
-            bool isWow64;
-            if (!IsWow64Process(process.Handle, out isWow64))
-            {
-                throw new Exception($"{nameof(IsWow64Process)} failed with {Marshal.GetLastWin32Error()}");
-            }
-
-            return !isWow64;
-        }
-
-        [DllImport("kernel32.dll", SetLastError = true, CallingConvention = CallingConvention.Winapi)]
-        [return: MarshalAs(UnmanagedType.Bool)]
-        private static extern bool IsWow64Process([In] IntPtr process, [Out] out bool wow64Process);
     }
 }

src/Tools/Source/RunTests/Program.cs

@@ -64,7 +64,7 @@ private static async Task<int> Run(Options options, CancellationToken cancellati
             var finishedTask = await Task.WhenAny(timeoutTask, runTask);
             if (finishedTask == timeoutTask)
             {
-                HandleTimeout(options);
+                await HandleTimeout(options, cancellationToken);
                 cts.Cancel();
 
                 try
@@ -150,26 +150,18 @@ private static void WriteLogFile(Options options)
         /// Invoked when a timeout occurs and we need to dump all of the test processes and shut down 
         /// the runnner.
         /// </summary>
-        private static void HandleTimeout(Options options)
+        private static async Task HandleTimeout(Options options, CancellationToken cancellationToken)
         {
-            string GetProcDumpFilePath(Process proc) => ProcessUtil.Is64Bit(proc)
-                ? Path.Combine(options.ProcDumpPath, "procdump64.exe")
-                : Path.Combine(options.ProcDumpPath, "procdump.exe");
+            var procDumpFilePath = Path.Combine(options.ProcDumpPath, "procdump.exe");
 
-            void DumpProcess(Process targetProcess, string dumpFilePath)
+            async Task DumpProcess(Process targetProcess, string dumpFilePath)
             {
                 Console.Write($"Dumping {targetProcess.ProcessName} {targetProcess.Id} to {dumpFilePath} ... ");
                 try
                 {
-                    var processStartInfo = new ProcessStartInfo();
-                    processStartInfo.FileName = GetProcDumpFilePath(targetProcess);
-                    processStartInfo.Arguments = $"-accepteula -ma {targetProcess.Id} {dumpFilePath}";
-                    processStartInfo.CreateNoWindow = true;
-                    processStartInfo.UseShellExecute = false;
-                    processStartInfo.WindowStyle = ProcessWindowStyle.Hidden;
-                    processStartInfo.RedirectStandardOutput = true;
-                    var process = Process.Start(processStartInfo);
-                    process.WaitForExit();
+                    var args = $"-accepteula -ma {targetProcess.Id} {dumpFilePath}";
+                    var processTask = ProcessRunner.RunProcessAsync(procDumpFilePath, args, cancellationToken);
+                    var processOutput = await processTask;
 
                     // The exit code for procdump doesn't obey standard windows rules.  It will return non-zero
                     // for succesful cases (possibly returning the count of dumps that were written).  Best 
@@ -180,15 +172,16 @@ void DumpProcess(Process targetProcess, string dumpFilePath)
                     }
                     else
                     {
-                        Console.WriteLine($"FAILED with {process.ExitCode}");
-                        Console.WriteLine($"{processStartInfo.FileName} {processStartInfo.Arguments}");
-                        Console.WriteLine(process.StandardOutput.ReadToEnd());
+                        Console.WriteLine($"FAILED with {processOutput.ExitCode}");
+                        Console.WriteLine($"{procDumpFilePath} {args}");
+                        Console.WriteLine(string.Join(Environment.NewLine, processOutput.OutputLines));
                     }
                 }
-                catch (Exception ex)
+                catch (Exception ex) when (!cancellationToken.IsCancellationRequested)
                 {
                     Console.WriteLine("FAILED");
                     Console.WriteLine(ex.Message);
+                    Logger.Log("Failed to dump process", ex);
                 }
             }
 
@@ -204,7 +197,7 @@ void DumpProcess(Process targetProcess, string dumpFilePath)
                 foreach (var proc in ProcessUtil.GetProcessTree(Process.GetCurrentProcess()).OrderBy(x => x.ProcessName))
                 {
                     var dumpFilePath = Path.Combine(dumpDir, $"{proc.ProcessName}-{counter}.dmp");
-                    DumpProcess(proc, dumpFilePath);
+                    await DumpProcess(proc, dumpFilePath);
                     counter++;
                 }
             }