From 1d908d197547e450b1ca716bd50da87011aead1c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 7 Jan 2021 16:10:59 +0100 Subject: [PATCH 1/3] add tests --- .../StandardErrorTests.cs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 tests/BenchmarkDotNet.IntegrationTests/StandardErrorTests.cs diff --git a/tests/BenchmarkDotNet.IntegrationTests/StandardErrorTests.cs b/tests/BenchmarkDotNet.IntegrationTests/StandardErrorTests.cs new file mode 100644 index 0000000000..e3efd532d3 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests/StandardErrorTests.cs @@ -0,0 +1,43 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Tests.Loggers; +using System; +using Xunit; +using Xunit.Abstractions; + +namespace BenchmarkDotNet.IntegrationTests +{ + public class StandardErrorTests : BenchmarkTestExecutor + { + private const string ErrorMessage = "ErrorMessage"; + + public StandardErrorTests(ITestOutputHelper output) : base(output) + { + } + + [Fact] + public void BenchmarkCanWriteToStandardError() => CanExecute(); + + public class WritingToStandardError + { + [Benchmark] + public void Write() => Console.Error.Write(new string('a', 10_000)); // the text needs to be big enough to hit the deadlock + } + + [Fact] + public void ExceptionMessageIsNotLost() + { + var logger = new OutputLogger(Output); + var config = CreateSimpleConfig(logger); + + CanExecute(config, fullValidation: false); + + Assert.Contains(ErrorMessage, logger.GetLog()); + } + + public class ThrowingException + { + [Benchmark] + public void Write() => throw new Exception(ErrorMessage); + } + } +} From 0d0d89d87c0740c766a85f43e221507b22049e47 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 7 Jan 2021 16:34:45 +0100 Subject: [PATCH 2/3] don't redirect Standard Error, as we don't read it and writing to it by benchmark can cause deadlocks. fixes #1629 --- .../Toolchains/DotNetCli/DotNetCliCommandExecutor.cs | 11 ++++++++--- .../Toolchains/DotNetCli/DotNetCliExecutor.cs | 3 ++- src/BenchmarkDotNet/Toolchains/Executor.cs | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs index 280a9dfee0..78558e191e 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs @@ -76,7 +76,7 @@ internal static string GetDotNetSdkVersion() } internal static ProcessStartInfo BuildStartInfo(string customDotNetCliPath, string workingDirectory, string arguments, - IReadOnlyList environmentVariables = null, bool redirectStandardInput = false) + IReadOnlyList environmentVariables = null, bool redirectStandardInput = false, bool redirectStandardError = true) { const string dotnetMultiLevelLookupEnvVarName = "DOTNET_MULTILEVEL_LOOKUP"; @@ -88,12 +88,17 @@ internal static ProcessStartInfo BuildStartInfo(string customDotNetCliPath, stri UseShellExecute = false, CreateNoWindow = true, RedirectStandardOutput = true, - RedirectStandardError = true, + RedirectStandardError = redirectStandardError, RedirectStandardInput = redirectStandardInput, - StandardErrorEncoding = Encoding.UTF8, StandardOutputEncoding = Encoding.UTF8, }; + if (redirectStandardError) // StandardErrorEncoding is only supported when standard error is redirected + { + startInfo.StandardErrorEncoding = Encoding.UTF8; + } + + if (environmentVariables != null) foreach (var environmentVariable in environmentVariables) startInfo.EnvironmentVariables[environmentVariable.Key] = environmentVariable.Value; diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs index 62ef814253..6e5fb8debc 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliExecutor.cs @@ -64,7 +64,8 @@ private ExecuteResult Execute(BenchmarkCase benchmarkCase, CustomDotNetCliPath, artifactsPaths.BinariesDirectoryPath, $"{executableName.Escape()} {benchmarkId.ToArguments()}", - redirectStandardInput: true); + redirectStandardInput: true, + redirectStandardError: false); // #1629 startInfo.SetEnvironmentVariables(benchmarkCase, resolver); diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs index 61d7ebfe36..1061c3fdd6 100644 --- a/src/BenchmarkDotNet/Toolchains/Executor.cs +++ b/src/BenchmarkDotNet/Toolchains/Executor.cs @@ -89,7 +89,7 @@ private ProcessStartInfo CreateStartInfo(BenchmarkCase benchmarkCase, ArtifactsP UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardInput = true, - RedirectStandardError = true, + RedirectStandardError = false, // #1629 CreateNoWindow = true, WorkingDirectory = null // by default it's null }; From a13bbf731ba5746b9338f13565fb08712e4b49eb Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 7 Jan 2021 17:04:52 +0100 Subject: [PATCH 3/3] extra line --- .../Toolchains/DotNetCli/DotNetCliCommandExecutor.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs index 78558e191e..979fc9c4e5 100644 --- a/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs +++ b/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommandExecutor.cs @@ -98,7 +98,6 @@ internal static ProcessStartInfo BuildStartInfo(string customDotNetCliPath, stri startInfo.StandardErrorEncoding = Encoding.UTF8; } - if (environmentVariables != null) foreach (var environmentVariable in environmentVariables) startInfo.EnvironmentVariables[environmentVariable.Key] = environmentVariable.Value;