Avoid hangs when starting benchmark process fails (#2207)

* add test

* add simple, but not 100% reliable fix

* use a fix that does not add a delay to every benchmark

Author: adamsitnik

src/BenchmarkDotNet/Loggers/Broker.cs

@@ -1,7 +1,10 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.IO.Pipes;
+using System.Threading;
+using System.Threading.Tasks;
 using BenchmarkDotNet.Diagnosers;
 using BenchmarkDotNet.Engines;
 using BenchmarkDotNet.Running;
@@ -11,28 +14,66 @@ namespace BenchmarkDotNet.Loggers
     internal class Broker
     {
         private readonly ILogger logger;
+        private readonly Process process;
         private readonly IDiagnoser diagnoser;
         private readonly AnonymousPipeServerStream inputFromBenchmark, acknowledgments;
         private readonly DiagnoserActionParameters diagnoserActionParameters;
+        private readonly ManualResetEvent finished;
 
         public Broker(ILogger logger, Process process, IDiagnoser diagnoser,
             BenchmarkCase benchmarkCase, BenchmarkId benchmarkId, AnonymousPipeServerStream inputFromBenchmark, AnonymousPipeServerStream acknowledgments)
         {
             this.logger = logger;
+            this.process = process;
             this.diagnoser = diagnoser;
             this.inputFromBenchmark = inputFromBenchmark;
             this.acknowledgments = acknowledgments;
             diagnoserActionParameters = new DiagnoserActionParameters(process, benchmarkCase, benchmarkId);
+            finished = new ManualResetEvent(false);
 
             Results = new List<string>();
             PrefixedOutput = new List<string>();
+
+            process.EnableRaisingEvents = true;
+            process.Exited += OnProcessExited;
         }
 
         internal List<string> Results { get; }
 
         internal List<string> PrefixedOutput { get; }
 
         internal void ProcessData()
+        {
+            // When the process fails to start, there is no pipe to read from.
+            // If we try to read from such pipe, the read blocks and BDN hangs.
+            // We can't use async methods with cancellation tokens because Anonymous Pipes don't support async IO.
+
+            // Usually, this property is not set yet.
+            if (process.HasExited)
+            {
+                return;
+            }
+
+            Task.Run(ProcessDataBlocking);
+
+            finished.WaitOne();
+            finished.Dispose();
+        }
+
+        private void OnProcessExited(object sender, EventArgs e)
+        {
+            process.Exited -= OnProcessExited;
+
+            // Dispose all the pipes to let reading from pipe finish with EOF and avoid a reasource leak.
+            inputFromBenchmark.DisposeLocalCopyOfClientHandle();
+            inputFromBenchmark.Dispose();
+            acknowledgments.DisposeLocalCopyOfClientHandle();
+            acknowledgments.Dispose();
+
+            finished.Set();
+        }
+
+        private void ProcessDataBlocking()
         {
             using StreamReader reader = new (inputFromBenchmark, AnonymousPipesHost.UTF8NoBOM, detectEncodingFromByteOrderMarks: false);
             using StreamWriter writer = new (acknowledgments, AnonymousPipesHost.UTF8NoBOM, bufferSize: 1);
@@ -67,6 +108,8 @@ internal void ProcessData()
                     {
                         // we have received the last signal so we can stop reading from the pipe
                         // if the process won't exit after this, its hung and needs to be killed
+                        process.Exited -= OnProcessExited;
+                        finished.Set();
                         return;
                     }
                 }

tests/BenchmarkDotNet.IntegrationTests/FailingProcessSpawnTests.cs

@@ -0,0 +1,44 @@
+using BenchmarkDotNet.Attributes;
+using BenchmarkDotNet.Environments;
+using BenchmarkDotNet.Jobs;
+using BenchmarkDotNet.Portability;
+using System.Linq;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace BenchmarkDotNet.IntegrationTests
+{
+    public class FailingProcessSpawnTests : BenchmarkTestExecutor
+    {
+        public FailingProcessSpawnTests(ITestOutputHelper output) : base(output)
+        {
+        }
+
+        [Fact]
+        public void NoHangs()
+        {
+            Platform wrongPlatform = RuntimeInformation.GetCurrentPlatform() switch
+            {
+                Platform.X64 or Platform.X86 => Platform.Arm64,
+                _ => Platform.X64
+            };
+
+            var invalidPlatformJob = Job.Dry.WithPlatform(wrongPlatform);
+            var config = CreateSimpleConfig(job: invalidPlatformJob);
+
+            var summary = CanExecute<Simple>(config, fullValidation: false);
+
+            var executeResults = summary.Reports.Single().ExecuteResults.Single();
+
+            Assert.True(executeResults.FoundExecutable);
+            Assert.False(executeResults.IsSuccess);
+            Assert.NotEqual(0, executeResults.ExitCode);
+        }
+
+        public class Simple
+        {
+            [Benchmark]
+            public void DoNothing() { }
+        }
+    }
+}