Skip to content

Commit

Permalink
Avoid hangs when starting benchmark process fails (#2207)
Browse files Browse the repository at this point in the history
* add test

* add simple, but not 100% reliable fix

* use a fix that does not add a delay to every benchmark
  • Loading branch information
adamsitnik committed Nov 29, 2022
1 parent 90c82bb commit 47b8b72
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 1 deletion.
45 changes: 44 additions & 1 deletion src/BenchmarkDotNet/Loggers/Broker.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
}
Expand Down
44 changes: 44 additions & 0 deletions tests/BenchmarkDotNet.IntegrationTests/FailingProcessSpawnTests.cs
Original file line number Diff line number Diff line change
@@ -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() { }
}
}
}

0 comments on commit 47b8b72

Please sign in to comment.