Skip to content

Commit 5b21673

Browse files
authored
don't redirect Standard Error, as we don't read it and writing to it by benchmark can cause deadlocks (#1631)
* add tests * don't redirect Standard Error, as we don't read it and writing to it by benchmark can cause deadlocks. fixes #1629
1 parent d5f7b9f commit 5b21673

File tree

4 files changed

+53
-5
lines changed

4 files changed

+53
-5
lines changed

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ internal static string GetDotNetSdkVersion()
7676
}
7777

7878
internal static ProcessStartInfo BuildStartInfo(string customDotNetCliPath, string workingDirectory, string arguments,
79-
IReadOnlyList<EnvironmentVariable> environmentVariables = null, bool redirectStandardInput = false)
79+
IReadOnlyList<EnvironmentVariable> environmentVariables = null, bool redirectStandardInput = false, bool redirectStandardError = true)
8080
{
8181
const string dotnetMultiLevelLookupEnvVarName = "DOTNET_MULTILEVEL_LOOKUP";
8282

@@ -88,12 +88,16 @@ internal static ProcessStartInfo BuildStartInfo(string customDotNetCliPath, stri
8888
UseShellExecute = false,
8989
CreateNoWindow = true,
9090
RedirectStandardOutput = true,
91-
RedirectStandardError = true,
91+
RedirectStandardError = redirectStandardError,
9292
RedirectStandardInput = redirectStandardInput,
93-
StandardErrorEncoding = Encoding.UTF8,
9493
StandardOutputEncoding = Encoding.UTF8,
9594
};
9695

96+
if (redirectStandardError) // StandardErrorEncoding is only supported when standard error is redirected
97+
{
98+
startInfo.StandardErrorEncoding = Encoding.UTF8;
99+
}
100+
97101
if (environmentVariables != null)
98102
foreach (var environmentVariable in environmentVariables)
99103
startInfo.EnvironmentVariables[environmentVariable.Key] = environmentVariable.Value;

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase,
6464
CustomDotNetCliPath,
6565
artifactsPaths.BinariesDirectoryPath,
6666
$"{executableName.Escape()} {benchmarkId.ToArguments()}",
67-
redirectStandardInput: true);
67+
redirectStandardInput: true,
68+
redirectStandardError: false); // #1629
6869

6970
startInfo.SetEnvironmentVariables(benchmarkCase, resolver);
7071

src/BenchmarkDotNet/Toolchains/Executor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ private ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsP
8989
UseShellExecute = false,
9090
RedirectStandardOutput = true,
9191
RedirectStandardInput = true,
92-
RedirectStandardError = true,
92+
RedirectStandardError = false, // #1629
9393
CreateNoWindow = true,
9494
WorkingDirectory = null // by default it's null
9595
};
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
using BenchmarkDotNet.Attributes;
2+
using BenchmarkDotNet.Tests.Loggers;
3+
using System;
4+
using Xunit;
5+
using Xunit.Abstractions;
6+
7+
namespace BenchmarkDotNet.IntegrationTests
8+
{
9+
public class StandardErrorTests : BenchmarkTestExecutor
10+
{
11+
private const string ErrorMessage = "ErrorMessage";
12+
13+
public StandardErrorTests(ITestOutputHelper output) : base(output)
14+
{
15+
}
16+
17+
[Fact]
18+
public void BenchmarkCanWriteToStandardError() => CanExecute<WritingToStandardError>();
19+
20+
public class WritingToStandardError
21+
{
22+
[Benchmark]
23+
public void Write() => Console.Error.Write(new string('a', 10_000)); // the text needs to be big enough to hit the deadlock
24+
}
25+
26+
[Fact]
27+
public void ExceptionMessageIsNotLost()
28+
{
29+
var logger = new OutputLogger(Output);
30+
var config = CreateSimpleConfig(logger);
31+
32+
CanExecute<ThrowingException>(config, fullValidation: false);
33+
34+
Assert.Contains(ErrorMessage, logger.GetLog());
35+
}
36+
37+
public class ThrowingException
38+
{
39+
[Benchmark]
40+
public void Write() => throw new Exception(ErrorMessage);
41+
}
42+
}
43+
}

0 commit comments

Comments
 (0)