17

I launch a process from C# as follows:

public bool Execute()
{
    ProcessStartInfo startInfo = new ProcessStartInfo();

    startInfo.Arguments =  "the command";
    startInfo.FileName = "C:\\MyApp.exe";

    startInfo.UseShellExecute = false;
    startInfo.RedirectStandardOutput = true;
    startInfo.RedirectStandardError = true;

    Log.LogMessage("{0} {1}", startInfo.FileName, startInfo.Arguments);

    using (Process myProcess = Process.Start(startInfo))
    {
        StringBuilder output = new StringBuilder();
        myProcess.OutputDataReceived += delegate(object sender, DataReceivedEventArgs e)
        {
            Log.LogMessage(Thread.CurrentThread.ManagedThreadId.ToString() + e.Data);
        };
        myProcess.ErrorDataReceived += delegate(object sender, DataReceivedEventArgs e)
        {
            Log.LogError(Thread.CurrentThread.ManagedThreadId.ToString() +  " " + e.Data);            
        };

        myProcess.BeginErrorReadLine();
        myProcess.BeginOutputReadLine();

        myProcess.WaitForExit();

    }

    return false;
}

But this has a problem... if the app in question writes to std out and std err in this order:

std out: msg 1
std err: msg 2
std out: msg 3

Then the output I see from the logs is:

msg 2
msg 1
msg 3

This seems to be because the event handlers are executed in another thread. So my question is how can the order of the process writing to std err and std out be maintained?

I thought of using a time stamp but I don't think this will work due to the preemptive nature of threads..

Update: Confirmed that using a time stamp on the data is no use.

Final update: The accepted answer solves this problem - however it does have one drawback, when the streams are merged there is no way to know which stream was written to. Hence if you require the logic of write to stderr == failure rather than the app exit code you might still be screwed.

paulm
  • 5,629
  • 7
  • 47
  • 70

2 Answers2

14

As far I understand, you want to preserve the order of stdout/stderr messages. I don't see any DECENT way to do this with C# managed Process(reflection - yes, nasty subclassing hacking - yes). It seems that it's pretty much hardcoded.

This functionality does not depend on threads themselves. If you want to keep the order, STDOUT and STDERROR have to use same handle(buffer). If they use the same buffer, it's going to be synchronized.

Here is a snippet from Process.cs:

 if (startInfo.RedirectStandardOutput) {
    CreatePipe(out standardOutputReadPipeHandle, 
               out startupInfo.hStdOutput, 
               false);
    } else {
    startupInfo.hStdOutput = new SafeFileHandle(
         NativeMethods.GetStdHandle(
                         NativeMethods.STD_OUTPUT_HANDLE), 
                         false);
}

if (startInfo.RedirectStandardError) {
    CreatePipe(out standardErrorReadPipeHandle, 
               out startupInfo.hStdError, 
               false);
    } else {
    startupInfo.hStdError = new SafeFileHandle(
         NativeMethods.GetStdHandle(
                         NativeMethods.STD_ERROR_HANDLE),
                         false);
}

as you can see, there are gonna be two buffers, and if we have two buffers, we have already lost the order information.

Basically, you need to create your own Process() class that can handle this case. Sad? Yes. The good news is that it's not hard, it seems pretty simple. Here is a code taken from StackOverflow, not C# but enough to understand the algorithm:

function StartProcessWithRedirectedOutput(const ACommandLine: string; const AOutputFile: string;
  AShowWindow: boolean = True; AWaitForFinish: boolean = False): Integer;
var
  CommandLine: string;
  StartupInfo: TStartupInfo;
  ProcessInformation: TProcessInformation;
  StdOutFileHandle: THandle;
begin
  Result := 0;

  StdOutFileHandle := CreateFile(PChar(AOutputFile), GENERIC_WRITE, FILE_SHARE_READ, nil, CREATE_ALWAYS,
    FILE_ATTRIBUTE_NORMAL, 0);
  Win32Check(StdOutFileHandle <> INVALID_HANDLE_VALUE);
  try
    Win32Check(SetHandleInformation(StdOutFileHandle, HANDLE_FLAG_INHERIT, 1));
    FillChar(StartupInfo, SizeOf(TStartupInfo), 0);
    FillChar(ProcessInformation, SizeOf(TProcessInformation), 0);

    StartupInfo.cb := SizeOf(TStartupInfo);
    StartupInfo.dwFlags := StartupInfo.dwFlags or STARTF_USESTDHANDLES;
    StartupInfo.hStdInput := GetStdHandle(STD_INPUT_HANDLE);
    StartupInfo.hStdOutput := StdOutFileHandle;
    StartupInfo.hStdError := StdOutFileHandle;

    if not(AShowWindow) then
    begin
      StartupInfo.dwFlags := StartupInfo.dwFlags or STARTF_USESHOWWINDOW;
      StartupInfo.wShowWindow := SW_HIDE;
    end;

    CommandLine := ACommandLine;
    UniqueString(CommandLine);

    Win32Check(CreateProcess(nil, PChar(CommandLine), nil, nil, True,
      CREATE_NEW_PROCESS_GROUP + NORMAL_PRIORITY_CLASS, nil, nil, StartupInfo, ProcessInformation));

    try
      Result := ProcessInformation.dwProcessId;

      if AWaitForFinish then
        WaitForSingleObject(ProcessInformation.hProcess, INFINITE);

    finally
      CloseHandle(ProcessInformation.hProcess);
      CloseHandle(ProcessInformation.hThread);
    end;

  finally
    CloseHandle(StdOutFileHandle);
  end;
end;

Source: How to redirect large amount of output from command executed by CreateProcess?

Instead of file, you want to use CreatePipe. From pipe, you can read asynchronously like so:

standardOutput = new StreamReader(new FileStream(
                       standardOutputReadPipeHandle, 
                       FileAccess.Read, 
                       4096, 
                       false),
                 enc, 
                 true, 
                 4096);

and BeginReadOutput()

  if (output == null) {
        Stream s = standardOutput.BaseStream;
        output = new AsyncStreamReader(this, s, 
          new UserCallBack(this.OutputReadNotifyUser), 
             standardOutput.CurrentEncoding);
    }
    output.BeginReadLine();
Community
  • 1
  • 1
Erti-Chris Eelmaa
  • 25,338
  • 6
  • 61
  • 78
  • Yeah, I'd agree with this answer. I did a bunch of research on this issue this afternoon and I think this is the right way to get it. Even if you were to use the underlying StreamReaders provided on Process for stdout/err, it sounds like the Peek method blocks because it doesn't use PeekNamedPipe under the hood. All this being said, I've been wondering exactly what you need to do. If all you care about is capturing stdout/stderr in the right order, not caring which is which, you could potentially create a batch file that uses the 2>&1 trick to push everything into stdout. Would that work? – J Trana Dec 26 '13 at 02:59
  • 1
    Ideally, I'd like to be able to get both `stdout` (for output processing) and `stdout+stderr` (for error reporting). Maybe, hook writes to the 2 handles with a common lock somehow... – ivan_pozdeev Dec 26 '13 at 08:18
  • In my case using a batch file with 2>&1 also captures it in the wrong order, I just don't think this is possible at all – paulm Dec 27 '13 at 08:25
  • @paulm: I don'tthink 2>&1 will work. Just tested it on my .NET application and compared GetStdHandle(STD_OUPUT) and GetStdHandle(STD_ERROR). Not sure how the batch pipe redirection works, but if you debug, you can see that it does not make 2 handles the same. Try rewriting the Pascal code, it should work for you, paulm. – Erti-Chris Eelmaa Dec 27 '13 at 12:08
  • If this does work then how do you know if the app wrote to stderr to detect there was a problem? I believe this must be why the .NET process class has the two pipes - the same as cmd.exe? – paulm Dec 27 '13 at 12:16
  • 2
    STDOUT/STDERR were never mean't to be ordered. As far I can see it right now: you either get one, or other. If you can run the same Process twice(one for stdout, other for stdout/stderr) then you will be fine. What you can do: Download ApiMonitor and look how console write is implemented under the hood(WriteOut). You can do process hijacking that will allow you to intercept ANY calls that are going on - thus allowing you to do anything you want. See for IAT API hooking in Google - not a cute way, but it will work. – Erti-Chris Eelmaa Dec 27 '13 at 12:25
  • What about using 3 handles, handle to stdout, stderr and stdout/stderr in one handle as you suggested? Would this allow the "correct" ordering and knowing if stderr was written to? – paulm Dec 29 '13 at 15:37
  • Has anyone had success of converting this (Delphi?) code to C#? – Michael Feb 03 '17 at 13:01
9

While I appreciate Erti-Chris's answer (what is that, Pascal?), I thought others might prefer an answer in a managed language. Also, to the detractors who say that "you shouldn't be doing this" because STDOUT and STDERR are not guaranteed to preserve the ordering: yes, I understand, but sometimes we have to interoperate with programs (that we did not write) that expect us to do just that, correct semantics be damned.

Here's a version in C#. Instead of circumventing the managed Process API by calling CreateProcess, it uses an alternative approach that redirects STDERR onto the STDOUT stream in the Windows shell. Because UseShellExecute = true does not actually use the cmd.exe shell (surprise!), you normally cannot use shell redirects. The workaround is to launch the cmd.exe shell ourselves, feeding our real shell program and arguments to it manually.

Note that the following solution assumes that your args array is already properly escaped. I like the brute force solution of using the kernel's GetShortPathName call, but you should know that it is not always appropriate to use (like if you're not on NTFS). Also, you really do want to go the extra step of reading the STDOUT buffer asynchronously (as I do below), because if you don't, your program may deadlock.

using System;
using System.Diagnostics;
using System.Text;
using System.Threading;

public static string runCommand(string cpath, string[] args)
{
    using (var p = new Process())
    {
        // notice that we're using the Windows shell here and the unix-y 2>&1
        p.StartInfo.FileName = @"c:\windows\system32\cmd.exe";
        p.StartInfo.Arguments = "/c \"" + cpath + " " + String.Join(" ", args) + "\" 2>&1";
        p.StartInfo.UseShellExecute = false;
        p.StartInfo.RedirectStandardOutput = true;
        p.StartInfo.RedirectStandardError = true;

        var output = new StringBuilder();

        using (var outputWaitHandle = new AutoResetEvent(false))
        {
            p.OutputDataReceived += (sender, e) =>
            {
                // attach event handler
                if (e.Data == null)
                {
                    outputWaitHandle.Set();
                }
                else
                {
                    output.AppendLine(e.Data);
                }
            };

            // start process
            p.Start();

            // begin async read
            p.BeginOutputReadLine();

            // wait for process to terminate
            p.WaitForExit();

            // wait on handle
            outputWaitHandle.WaitOne();

            // check exit code
            if (p.ExitCode == 0)
            {
                return output.ToString();
            }
            else
            {
                throw new Exception("Something bad happened");
            }
        }
    }
}
Community
  • 1
  • 1
Dan Barowy
  • 2,270
  • 24
  • 35
  • Thanks, this is actually a very simple approach. In my case my process happened to be a batch file anyway, so this really incurs no added overhead or complications. – StayOnTarget Dec 07 '16 at 13:07
  • 2
    To clarify, the `AutoResetEvent` and the `e.Data == null` are there because `p.WaitForExit` can happen before the last `OutputDataReceived` event. But when the output stream is closed, a final event is sent with `e.Data == null`. Source: https://msdn.microsoft.com/en-us/library/system.diagnostics.datareceivedeventhandler%28v=vs.110%29.aspx – Kjell Rilbe Sep 21 '17 at 15:20