Process.ErrorDataReceived fired after Process is disposed?

Question

I have my code below and sometimes get ObjectDisposedException at errorWaitHandle.Set();.

How could this happen when my process instance is disposed?

System.ObjectDisposedException: Safe handle has been closed

public static int Execute(string filename, string arguments, out string output, out string error, int timeoutInMilliSeconds = Timeout.Infinite)
    {
        using (AutoResetEvent outputWaitHandle = new AutoResetEvent(false), errorWaitHandle = new AutoResetEvent(false))
        {
            // separate using for process to ensure this is disposed before handles above.
            using (System.Diagnostics.Process process = new System.Diagnostics.Process())
            {
                process.StartInfo.FileName = filename;
                process.StartInfo.Arguments = arguments;
                process.StartInfo.UseShellExecute = false;
                process.StartInfo.RedirectStandardOutput = true;
                process.StartInfo.RedirectStandardError = true;

                StringBuilder outputSB = new StringBuilder();
                StringBuilder errorSB = new StringBuilder();

                process.OutputDataReceived += (sender, e) =>
                {
                    if (e.Data == null)
                    {
                        outputWaitHandle.Set();
                    }
                    else
                    {
                        outputSB.AppendLine(e.Data);
                    }
                };
                process.ErrorDataReceived += (sender, e) =>
                {
                    if (e.Data == null)
                    {
                        errorWaitHandle.Set();
                    }
                    else
                    {
                        errorSB.AppendLine(e.Data);
                    }
                };

                process.Start();

                // See http://stackoverflow.com/questions/139593/processstartinfo-hanging-on-waitforexit-why
                // for why we need to read output and error asynch
                process.BeginOutputReadLine();
                process.BeginErrorReadLine();

                if (!process.WaitForExit(timeoutInMilliSeconds) ||
                    !outputWaitHandle.WaitOne(timeoutInMilliSeconds) ||
                    !errorWaitHandle.WaitOne(timeoutInMilliSeconds))
                {
                    throw new TimeoutException(
                        string.Format("Executing [{0}] with argument [{1}] didn't finish within timeout {2} milliseconds", filename, arguments, timeoutInMilliSeconds));
                }

                output = outputSB.ToString();
                error = errorSB.ToString();

                return process.ExitCode;
            }
        }
    }

Answer 1

I've found that the Process events can fire in unexpected orders because of their asynchronous nature (i.e. "Exited" being fired BEFORE "ErrorDataReceived").

You also don't know how those events are being wired up under the covers of the Process class so the various object(s) lifetimes are not really known to you. By the time your handler gets called, the Process object could have been (and apparently is) disposed.

I tried to approach this problem almost identically as you; by using AutoResetEvent's and building up the Error / Data strings from within their respective event handlers.

The way I ended up fixing this is by calling Process.WaitForExit() twice:

System.Diagnostics.Process process = new System.Diagnostics.Process()

// Process setup code
if(process.WaitForExit(timeout)){

    process.WaitForExit(); // Note the lack of a timeout parameter

    // By now all your events should have fired and your strings built
    string errorString = errorSB.ToString();
}

The excerpt from MSDN states:

When standard output has been redirected to asynchronous event handlers, it is possible that output processing will not have completed when this method returns. To ensure that asynchronous event handling has been completed, call the WaitForExit() overload that takes no parameter after receiving a true from this overload. To help ensure that the Exited event is handled correctly in Windows Forms applications, set the SynchronizingObject property.

Source: https://msdn.microsoft.com/en-us/library/ty0d8k56(v=vs.110)

Answer 2

The solution is to subscribe to OutputDataReceived and ErrorDataReceived events to actual methods instead of anonymous methods. This way you can unsubscribe in Dispose() method.

See full code here:

https://github.com/borismod/OsTestFramework/blob/master/OsTestFramework/ProcessExecutor.cs