From eacd541651ddd4b26f5d909110c8736ac9cc67b0 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Mon, 30 Apr 2018 07:31:59 -0400 Subject: [PATCH] Fix for https://github.com/madelson/MedallionShell/issues/22 --- .../PlatformCompatibilityTest.cs | 9 ++++ MedallionShell/MedallionShell.csproj | 4 +- MedallionShell/PlatformCompatibilityHelper.cs | 40 ++++++++++++++++++ MedallionShell/ProcessCommand.cs | 17 ++++---- SampleCommand/PlatformCompatibilityTests.cs | 41 +++++++++++++++++++ 5 files changed, 100 insertions(+), 11 deletions(-) diff --git a/MedallionShell.Tests/PlatformCompatibilityTest.cs b/MedallionShell.Tests/PlatformCompatibilityTest.cs index 01163d1..4779ff3 100644 --- a/MedallionShell.Tests/PlatformCompatibilityTest.cs +++ b/MedallionShell.Tests/PlatformCompatibilityTest.cs @@ -18,9 +18,18 @@ public class PlatformCompatibilityTest [TestMethod] public void TestWriteAfterExit() => RunTest(() => PlatformCompatibilityTests.TestWriteAfterExit()); + [TestMethod] + public void TestFlushAfterExit() => RunTest(() => PlatformCompatibilityTests.TestFlushAfterExit()); + [TestMethod] public void TestExitWithMinusOne() => RunTest(() => PlatformCompatibilityTests.TestExitWithMinusOne()); + [TestMethod] + public void TestExitWithOne() => RunTest(() => PlatformCompatibilityTests.TestExitWithOne()); + + [TestMethod] + public void TestBadProcessFile() => RunTest(() => PlatformCompatibilityTests.TestBadProcessFile()); + private static void RunTest(Expression testMethod) { var compiled = testMethod.Compile(); diff --git a/MedallionShell/MedallionShell.csproj b/MedallionShell/MedallionShell.csproj index b022859..8fb03ba 100644 --- a/MedallionShell/MedallionShell.csproj +++ b/MedallionShell/MedallionShell.csproj @@ -6,7 +6,7 @@ Medallion.Shell - 1.5.0 + 1.5.1 Michael Adelson A lightweight library that simplifies working with processes in .NET Copyright © 2017 Michael Adelson @@ -16,7 +16,7 @@ https://github.com/madelson/MedallionShell 1.0.0.0 - Added ToString() override for Commands to simplify debugging. WindowsCommandLineSyntax no longer quotes arguments unnecessarily. + See https://github.com/madelson/MedallionShell#release-notes diff --git a/MedallionShell/PlatformCompatibilityHelper.cs b/MedallionShell/PlatformCompatibilityHelper.cs index dca215c..361865d 100644 --- a/MedallionShell/PlatformCompatibilityHelper.cs +++ b/MedallionShell/PlatformCompatibilityHelper.cs @@ -35,5 +35,45 @@ public static int SafeGetExitCode(this Process process) return process.ExitCode; } + + /// + /// Starts the given and captures the standard IO streams. This method works around Mono.Android-specific + /// issue https://github.com/madelson/MedallionShell/issues/22, where a process that exits quickly causes the initialization of + /// the standard input writer to fail (since setting AutoFlush = true triggers a write which on Mono crashes for a closed process). + /// + /// If https://github.com/mono/mono/issues/8478 is ever addressed, we wouldn't need this any more. + /// + public static bool SafeStart(this Process process, out StreamWriter standardInput, out StreamReader standardOutput, out StreamReader standardError) + { + var redirectStandardInput = process.StartInfo.RedirectStandardInput; + var redirectStandardOutput = process.StartInfo.RedirectStandardOutput; + var redirectStandardError = process.StartInfo.RedirectStandardError; + + try + { + process.Start(); + + // adding this code allows for a sort-of replication of + // https://github.com/madelson/MedallionShell/issues/22 on non-Android platforms + //process.StandardInput.BaseStream.Write(new byte[1000], 0, 1000); + //process.StandardInput.BaseStream.Flush(); + } + catch (IOException ex) + // note that AFAIK the exact type check here isn't necessary, but it seems more robust against + // other types of IOExceptions (e. g. FileNotFoundException, PathTooLongException) that could in + // theory be thrown here and trigger this + when (IsMono && ex.GetType() == typeof(IOException)) + { + standardInput = redirectStandardInput ? new StreamWriter(Stream.Null, Console.InputEncoding) { AutoFlush = true } : null; + standardOutput = redirectStandardOutput ? new StreamReader(Stream.Null, Console.OutputEncoding) : null; + standardError = redirectStandardError ? new StreamReader(Stream.Null, Console.OutputEncoding) : null; + return false; + } + + standardInput = redirectStandardInput ? process.StandardInput : null; + standardOutput = redirectStandardOutput ? process.StandardOutput : null; + standardError = redirectStandardError ? process.StandardError : null; + return true; + } } } diff --git a/MedallionShell/ProcessCommand.cs b/MedallionShell/ProcessCommand.cs index 8e6bd39..2d45e8b 100644 --- a/MedallionShell/ProcessCommand.cs +++ b/MedallionShell/ProcessCommand.cs @@ -36,17 +36,17 @@ internal sealed class ProcessCommand : Command var processMonitoringTask = CreateProcessMonitoringTask(this.process); - this.process.Start(); - + this.process.SafeStart(out var processStandardInput, out var processStandardOutput, out var processStandardError); + var ioTasks = new List(capacity: 2); if (startInfo.RedirectStandardOutput) { - this.standardOutputReader = new InternalProcessStreamReader(this.process.StandardOutput); + this.standardOutputReader = new InternalProcessStreamReader(processStandardOutput); ioTasks.Add(this.standardOutputReader.Task); } if (startInfo.RedirectStandardError) { - this.standardErrorReader = new InternalProcessStreamReader(this.process.StandardError); + this.standardErrorReader = new InternalProcessStreamReader(processStandardError); ioTasks.Add(this.standardErrorReader.Task); } if (startInfo.RedirectStandardInput) @@ -54,11 +54,10 @@ internal sealed class ProcessCommand : Command // unfortunately, changing the encoding can't be done via ProcessStartInfo so we have to do it manually here. // See https://github.com/dotnet/corefx/issues/20497 - var originalStandardInput = this.process.StandardInput; - var wrappedStream = PlatformCompatibilityHelper.WrapStandardInputStreamIfNeeded(originalStandardInput.BaseStream); - var standardInputEncodingToUse = standardInputEncoding ?? originalStandardInput.Encoding; - var streamWriter = wrappedStream == originalStandardInput.BaseStream && Equals(standardInputEncodingToUse, originalStandardInput.Encoding) - ? originalStandardInput + var wrappedStream = PlatformCompatibilityHelper.WrapStandardInputStreamIfNeeded(processStandardInput.BaseStream); + var standardInputEncodingToUse = standardInputEncoding ?? processStandardInput.Encoding; + var streamWriter = wrappedStream == processStandardInput.BaseStream && Equals(standardInputEncodingToUse, processStandardInput.Encoding) + ? processStandardInput : new StreamWriter(wrappedStream, standardInputEncodingToUse); this.standardInput = new ProcessStreamWriter(streamWriter); } diff --git a/SampleCommand/PlatformCompatibilityTests.cs b/SampleCommand/PlatformCompatibilityTests.cs index 61e470c..b651f6b 100644 --- a/SampleCommand/PlatformCompatibilityTests.cs +++ b/SampleCommand/PlatformCompatibilityTests.cs @@ -1,6 +1,8 @@ using Medallion.Shell; using System; using System.Collections.Generic; +using System.ComponentModel; +using System.IO; using System.Linq; using System.Text; using System.Threading.Tasks; @@ -16,6 +18,16 @@ public static void TestWriteAfterExit() var command = Command.Run(SampleCommandPath, "exit", 1); command.Wait(); command.StandardInput.WriteLine(); // no-op + command.StandardInput.BaseStream.WriteAsync(new byte[1], 0, 1).Wait(); // no-op + } + + public static void TestFlushAfterExit() + { + var command = Command.Run(SampleCommandPath, "exit", 1); + command.Wait(); + command.StandardInput.Flush(); + command.StandardInput.BaseStream.Flush(); + command.StandardInput.BaseStream.FlushAsync(); } public static void TestReadAfterExit() @@ -40,5 +52,34 @@ public static void TestExitWithMinusOne() var command = Command.Run(SampleCommandPath, "exit", -1); if (command.Result.ExitCode != -1) { throw new InvalidOperationException($"Was: {command.Result.ExitCode}"); } } + + /// + /// See PlatformCompatibilityHelper.SafeStart comment + /// + public static void TestExitWithOne() + { + var command = Command.Run(SampleCommandPath, "exit", 1); + if (command.Result.ExitCode != 1) { throw new InvalidOperationException($"Was: {command.Result.ExitCode}"); } + } + + public static void TestBadProcessFile() + { + var baseDirectory = Path.GetDirectoryName(SampleCommandPath); + + AssertThrows(() => Command.Run(baseDirectory)); + AssertThrows(() => Command.Run(Path.Combine(baseDirectory, "DOES_NOT_EXIST.exe"))); + } + + private static void AssertThrows(Action action) where TException : Exception + { + try { action(); } + catch (Exception ex) + { + if (ex.GetType() != typeof(TException)) { throw new InvalidOperationException($"Expected {typeof(TException)} but got {ex.GetType()}"); } + return; + } + + throw new InvalidOperationException($"Expected {typeof(TException)}, but no exception was thrown"); + } } }