Skip to content

Commit

Permalink
Fix for #22
Browse files Browse the repository at this point in the history
  • Loading branch information
madelson committed Apr 30, 2018
1 parent ac89d81 commit eacd541
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 11 deletions.
9 changes: 9 additions & 0 deletions MedallionShell.Tests/PlatformCompatibilityTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Action> testMethod)
{
var compiled = testMethod.Compile();
Expand Down
4 changes: 2 additions & 2 deletions MedallionShell/MedallionShell.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<PropertyGroup>
<RootNamespace>Medallion.Shell</RootNamespace>
<Version>1.5.0</Version>
<Version>1.5.1</Version>
<Authors>Michael Adelson</Authors>
<Description>A lightweight library that simplifies working with processes in .NET</Description>
<Copyright>Copyright © 2017 Michael Adelson</Copyright>
Expand All @@ -16,7 +16,7 @@
<PackageProjectUrl>https://github.com/madelson/MedallionShell</PackageProjectUrl>
<RepositoryUrl />
<FileVersion>1.0.0.0</FileVersion>
<PackageReleaseNotes>Added ToString() override for Commands to simplify debugging. WindowsCommandLineSyntax no longer quotes arguments unnecessarily.</PackageReleaseNotes>
<PackageReleaseNotes>See https://github.com/madelson/MedallionShell#release-notes</PackageReleaseNotes>
</PropertyGroup>

<PropertyGroup>
Expand Down
40 changes: 40 additions & 0 deletions MedallionShell/PlatformCompatibilityHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,45 @@ public static int SafeGetExitCode(this Process process)

return process.ExitCode;
}

/// <summary>
/// Starts the given <paramref name="process"/> 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.
/// </summary>
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;
}
}
}
17 changes: 8 additions & 9 deletions MedallionShell/ProcessCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,29 +36,28 @@ internal ProcessCommand(

var processMonitoringTask = CreateProcessMonitoringTask(this.process);

this.process.Start();

this.process.SafeStart(out var processStandardInput, out var processStandardOutput, out var processStandardError);
var ioTasks = new List<Task>(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)
{
// 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);
}
Expand Down
41 changes: 41 additions & 0 deletions SampleCommand/PlatformCompatibilityTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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()
Expand All @@ -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}"); }
}

/// <summary>
/// See PlatformCompatibilityHelper.SafeStart comment
/// </summary>
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<Win32Exception>(() => Command.Run(baseDirectory));
AssertThrows<Win32Exception>(() => Command.Run(Path.Combine(baseDirectory, "DOES_NOT_EXIST.exe")));
}

private static void AssertThrows<TException>(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");
}
}
}

0 comments on commit eacd541

Please sign in to comment.