How to close cmd.exe window while its being ran as

Question

I am very new to C# and trying to understand background workers.

Currently when I run this code it will stop redirecting and reading the output from the command prompt after the StopButton has been clicked and leaves the "cancelled" message behind, but then does nothing afterwards. I currently may be implementing this all wrong, but I have the e.Cancel set by clicking the stop button that calls CancelAsync(), which turns CancellationPending = true. Anyone have any ideas how I should go about this?

Thanks so much!! I appreciate the help!

    public Form2()
    {
        InitializeComponent();

        bw.WorkerReportsProgress = true;
        bw.WorkerSupportsCancellation = true;
        bw.DoWork += new DoWorkEventHandler(bw_DoWork);
        bw.RunWorkerCompleted += new   
        RunWorkerCompletedEventHandler(bw_RunWorkerCompleted);

    }

    private void StopButton_Click(object sender, EventArgs e)
    {
        if (bw.WorkerSupportsCancellation == true)
        {
            bw.CancelAsync();
            bw.Dispose();

            Invoke(new ToDoDelegate(() => this.textBox2.Text += Environment.NewLine + "Cancelled " + Environment.NewLine));
        }
    }

    private void Submit_Click(object sender, EventArgs e)
    {
            if(bw.IsBusy != true)
            {
                bw.RunWorkerAsync();
                Invoke(new ToDoDelegate(() => this.textBox2.Text += "Starting Download " + Environment.NewLine));
            }

    }

    public bool DoSVNCheckout(String KeyUrl, DoWorkEventArgs e)
    {
        SVNProcess = new Process
        {
            StartInfo = new ProcessStartInfo
            {

                FileName = "cmd.exe",
                Arguments = "/C plink download files using svn"
                Verb = "runas",
                UseShellExecute = false,
                RedirectStandardOutput = true,
                RedirectStandardError = true,
                CreateNoWindow = false,
            }
        };

        SVNProcess.Start();
        while(!SVNProcess.StandardOutput.EndOfStream & bw.CancellationPending == false)
        {
            string output = SVNProcess.StandardOutput.ReadLine();
            Invoke(new ToDoDelegate(() => this.textBox2.Text += output));
        }
        while (!SVNProcess.StandardError.EndOfStream & bw.CancellationPending == false)
        {
            string Erroutput = SVNProcess.StandardError.ReadLine();
            Invoke(new ToDoDelegate(() => this.textBox2.Text += Erroutput));
        }
        if(SVNProcess.StandardError.EndOfStream & bw.CancellationPending == false)
        {
            string Erroutput = SVNProcess.StandardError.ReadLine();
            Invoke(new ToDoDelegate(() => this.textBox2.Text += Erroutput));
        }

        //if I manually close the cmd.exe window by clicking X
        //in the top right corner the program runs correctly
        //at these lines of code right here

        if(bw.CancellationPending == true)
        {
            e.Cancel = true;
            return true;
        }
        return false;
    }

    private delegate void ToDoDelegate();
    private void bw_DoWork(object sender, DoWorkEventArgs e)
    {
        BackgroundWorker worker = sender as BackgroundWorker;


            if(bw.CancellationPending == true)
            {
                e.Cancel = true;
                return;
            }
            e.Cancel = DoSVNCheckout(URL, e);

    }
    private void bw_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
    {

        if ((e.Cancelled == true))
        {
            this.textBox2.Text = "Canceled!";
        }
        else{
            this.textBox2.Text = "Done!";
        }
    }

Answer 1

There are a number of problems with the code you've written. The two primary issues, in my opinion are:

1. First and foremost, you seem to be confusing the BackgroundWorker operation with the separately-running process you've started. The two are not by any means the same, or even related to each other. Cancelling the BackgroundWorker won't have any direct effect on the process you start. Your question is not clear on what the actual desired behavior is here, but you don't do anything to actually terminate the external process. At best, if you weren't blocking the DoWork method waiting for the process to produce output, you'd abandon the process. As it is, without terminating the process, your DoWork can never get to a point where it notices you've tried to cancel it, because it's stuck on a ReadLine() call.
2. You are consuming the StandardOutput and StandardError streams in serial fashion, i.e. one after the other. The documentation clearly warns against this, as this is a very reliable way to deadlock your code. The buffers for each stream are relatively small, and the entire external process will hang if it tries to write to one of these streams when the buffer is full. This will in turn cause no more output to be written to any of the streams. In your code example, if the StandardError stream buffer gets full before you have been able to completely read the StandardOutput stream, the external process will hang and your own process will as well.

Another minor issue is that you don't take advantage of the BackgroundWorker.ProgressChanged event, which you could have used to pass text you've read from the output and error strings back to the UI thread where you could add that text to your text box. The use of Control.Invoke() here isn't strictly necessary, and fails to fully take advantage of the features in BackgroundWorker.

There are ways to modify the code you've written, so that you can still use BackgroundWorker and achieve your goal. One obvious improvement that you could make would be to store the Process object reference in an instance field, so that it's accessible by the StopButton_Click() method. In that method, you could call the Process.Kill() method to actually terminate the process.

But even so, you would need to fix the deadlock-prone implementation you've got now. This can be done in a variety of ways: use the Process.OutputDataReceived and Process.ErrorDataReceived events; create a second BackgroundWorker task to process one of the streams; use the Task-based idioms to read the streams.

The last option there would be my preference. The second option needlessly creates long-running threads, while the event-based pattern (the first option) is awkward to use (and is line-based, so has limited value when dealing with processes that write partial lines during the course of their operation). But if you're going to use the Task-based idioms for reading the streams, it seems to me you should upgrade the entire implementation to do so.

BackgroundWorker is still a viable class to use if one wants to, but the new Task features in conjunction with the async/await keywords offer what is IMHO a much easier and cleaner way to handle asynchronous operations. One of the biggest advantages is that it doesn't rely on explicitly used threads (e.g. running the DoWork event handler in a thread pool thread). Asynchronous I/O operations such as what makes up the entirety of your scenario here, are handled implicitly via the API, allowing all of the code you write to be executed in the UI thread where you want it.

Here is a version of your example that does just this:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
    }

    private TaskCompletionSource<bool> _cancelTask;

    private async void button1_Click(object sender, EventArgs e)
    {
        button1.Enabled = false;
        button2.Enabled = true;
        _cancelTask = new TaskCompletionSource<bool>();

        try
        {
            await RunProcess();
        }
        catch (TaskCanceledException)
        {
            MessageBox.Show("The operation was cancelled");
        }
        finally
        {
            _cancelTask = null;
            button1.Enabled = true;
            button2.Enabled = false;
        }
    }

    private void button2_Click(object sender, EventArgs e)
    {
        _cancelTask.SetCanceled();
    }

    private async Task RunProcess()
    {
        Process process = new Process
        {
            StartInfo = new ProcessStartInfo
            {
                FileName = "cmd.exe",
                Arguments = "/C pause",
                UseShellExecute = false,
                RedirectStandardOutput = true,
                RedirectStandardError = true,
                CreateNoWindow = false,
            }
        };

        process.Start();

        Task readerTasks = Task.WhenAll(
            ConsumeReader(process.StandardError),
            ConsumeReader(process.StandardOutput));

        Task completedTask = await Task.WhenAny(readerTasks, _cancelTask.Task);

        if (completedTask == _cancelTask.Task)
        {
            process.Kill();
            await readerTasks;
            throw new TaskCanceledException(_cancelTask.Task);
        }
    }

    private async Task ConsumeReader(TextReader reader)
    {
        char[] text = new char[512];
        int cch;

        while ((cch = await reader.ReadAsync(text, 0, text.Length)) > 0)
        {
            textBox1.AppendText(new string(text, 0, cch));
        }
    }
}

Notes:

1. First, as you can see, there is no longer a need for BackgroundWorker at all. The async/await pattern implicitly does all of the same work that BackgroundWorker would have for you, but without all the extra boilerplate code required to set it up and manage it.
2. There is a new instance field, _cancelTask, which represents a simple Task object that can be completed. It's only ever completed by being cancelled, in this case, but that's not strictly required…you'll note that the await statement that monitors the task completion doesn't actually care how the task ended. Just that it did. In more complex scenarios, one might actually want to use the Result for such a Task object, calling SetResult() to complete the task with a value, and using SetCanceled() for actually cancelling the operation being represented. It all depends on the specific context.
3. The button1_Click() method (equivalent to your Submit_Click() method) is written as if everything happens synchronously. Through the "magic" of the await statement, the method actually executes in two parts. When you click the button, everything up to the await statement executes; at the await, once the RunProcess() method has returned, the button1_Click() method returns. It will resume execution later, when the Task object returned by RunProcess() completes, which in turn will happen when that method reaches its end (i.e. not the first time it returns).
4. In the button1_Click() method, the UI is updated to reflect the current state of operation: the start button is disabled and the cancel button is enabled. Before returning, the buttons are returned to their original state.
5. The button1_Click() method is also where the _cancelTask object is created and later discarded. The await RunProcess() statement will see a TaskCanceledException if the RunProcess() throws it; this is used to present the user with the MessageBox reporting that the operation had been cancelled. You can of course respond to an exception like this however you see fit.
6. In this way, the button2_Click() method (equivalent to your StopButton_Click() method) only has to set the _cancelTask object to a completed state, in this case by calling SetCanceled().
7. The RunProcess() method is where the main handling of the process occurs. It starts the process, then waits for the relevant tasks to complete. The two tasks representing the output and error streams are wrapping in a call to Task.WhenAll(). This creates a new Task object that will complete only when all of the wrapped tasks are completed. Then the method waits via Task.WhenAny() for that wrapper task and the _cancelTask object. If either completes, the method will complete execution. If the completed task was the _cancelTask object, the method does so by killing the process that was started (interrupting it in the middle of whatever it was doing), waiting for the processs to actually exit (which can be observed by the completion of the wrapper task…these complete when the end of both the output and the error streams is reached, which in turn happens when the process has exited), and then throwing the TaskCanceledException.
8. The ConsumeReader() method is a helper method that simple reads text from the given TextReader object, and appends the output to the text box. It uses TextReader.ReadAsync(); this type of method can also be written using TextReader.ReadLineAsync(), but in that case you will only see output at the end of each line. Using ReadAsync() ensures that output is retrieved as soon as it's available, without waiting for newline characters.
9. Note that the RunProcess() and ConsumeReader() methods are also async, and also have await statements in them. As with button1_Click(), these methods initially return from executing upon reaching the await statement, resuming execution later when the awaited Task completes. In the ConsumeReader() example, you'll also notice that await unpacks the int value that is the Result property value of the Task<int> it was waiting on. The await statement forms an expression that evaluates to the waited Task's Result value.
10. A very important characteristic of the use of await in each of these cases is that the framework resumes execution of the method on the UI thread. This is why the button1_Click() method can still access the UI objects button1 and button2 after the await, and why ConsumeReader() can access the textBox1 object for the purpose of appending text each time some is returned by the ReadAsync() method.

I realize the above might be a lot to digest. Especially when most of it relates to the complete change from the use of BackgroundWorker to the Task-based API, rather than addressing the main two issues I mentioned at the outset. But I hope you can see how those issues are implicitly addressed by these changes, as well as how other requirements of the code are met in a simpler, easier-to-read fashion by using the modern async/await pattern.

For completeness, here is the Designer-generated code that goes along with the above Form1 class:

partial class Form1
{
    /// <summary>
    /// Required designer variable.
    /// </summary>
    private System.ComponentModel.IContainer components = null;

    /// <summary>
    /// Clean up any resources being used.
    /// </summary>
    /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
    protected override void Dispose(bool disposing)
    {
        if (disposing && (components != null))
        {
            components.Dispose();
        }
        base.Dispose(disposing);
    }

    #region Windows Form Designer generated code

    /// <summary>
    /// Required method for Designer support - do not modify
    /// the contents of this method with the code editor.
    /// </summary>
    private void InitializeComponent()
    {
        this.button1 = new System.Windows.Forms.Button();
        this.button2 = new System.Windows.Forms.Button();
        this.textBox1 = new System.Windows.Forms.TextBox();
        this.SuspendLayout();
        // 
        // button1
        // 
        this.button1.Location = new System.Drawing.Point(12, 12);
        this.button1.Name = "button1";
        this.button1.Size = new System.Drawing.Size(75, 23);
        this.button1.TabIndex = 0;
        this.button1.Text = "Start";
        this.button1.UseVisualStyleBackColor = true;
        this.button1.Click += new System.EventHandler(this.button1_Click);
        // 
        // button2
        // 
        this.button2.Enabled = false;
        this.button2.Location = new System.Drawing.Point(93, 12);
        this.button2.Name = "button2";
        this.button2.Size = new System.Drawing.Size(75, 23);
        this.button2.TabIndex = 0;
        this.button2.Text = "Stop";
        this.button2.UseVisualStyleBackColor = true;
        this.button2.Click += new System.EventHandler(this.button2_Click);
        // 
        // textBox1
        // 
        this.textBox1.Anchor = ((System.Windows.Forms.AnchorStyles)((((System.Windows.Forms.AnchorStyles.Top | System.Windows.Forms.AnchorStyles.Bottom) 
        | System.Windows.Forms.AnchorStyles.Left) 
        | System.Windows.Forms.AnchorStyles.Right)));
        this.textBox1.Location = new System.Drawing.Point(13, 42);
        this.textBox1.Multiline = true;
        this.textBox1.Name = "textBox1";
        this.textBox1.ReadOnly = true;
        this.textBox1.ScrollBars = System.Windows.Forms.ScrollBars.Vertical;
        this.textBox1.Size = new System.Drawing.Size(488, 258);
        this.textBox1.TabIndex = 1;
        // 
        // Form1
        // 
        this.AutoScaleDimensions = new System.Drawing.SizeF(8F, 16F);
        this.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font;
        this.ClientSize = new System.Drawing.Size(513, 312);
        this.Controls.Add(this.textBox1);
        this.Controls.Add(this.button2);
        this.Controls.Add(this.button1);
        this.Name = "Form1";
        this.Text = "Form1";
        this.ResumeLayout(false);
        this.PerformLayout();

    }

    #endregion

    private System.Windows.Forms.Button button1;
    private System.Windows.Forms.Button button2;
    private System.Windows.Forms.TextBox textBox1;
}