Why does my loop use 100% CPU and never end?

Question

I have this method:

    private delegate void watcherReader(StreamReader sr);
    private void watchProc(StreamReader sr) {
        while (true) {
            string line = sr.ReadLine();
            while (line != null) {
                if (stop) {
                    return;
                }
                //Console.WriteLine(line);
                line = stripColors(line);
                txtOut.Text += line + "\n";

                line = sr.ReadLine();
            }
        }
    }

And it reads the streams from a Process (cmd.exe). When the user closes the cmd.exe window, it causes the CPU usage to jump to 100%. When playing with the debugger I see that it stops on the sr.ReadLine() and never returns. Because this is watching both the StandardErrorStream and the StandardOutputStream it uses 100% on both cores.

Here's some more code of the project if you need it.

    [DllImport("User32")]
    private static extern int ShowWindow(int hwnd, int nCmdShow);   //this will allow me to hide a window

    public ConsoleForm(Process p) {
        this.p = p;
        p.Start();
        ShowWindow((int)p.MainWindowHandle, 0);   //0 means to hide the window.

        this.inStream = p.StandardInput;
        this.outStream = p.StandardOutput;
        this.errorStream = p.StandardError;

        InitializeComponent();

        wr = new watcherReader(watchProc);
        wr.BeginInvoke(this.outStream, null, null);
        wr.BeginInvoke(this.errorStream, null, null);
    }

    public void start(string[] folders, string serverPath) {

        this.inStream.WriteLine("chdir C:\\cygwin\\bin");
        this.inStream.WriteLine("bash --login -i");
        this.inStream.WriteLine("");
    }


    //code example from http://geekswithblogs.net/Waynerds/archive/2006/01/29/67506.aspx it is
    //to make the textbox autoscroll I don't understand what it does, but it works.
    #region autoscroll
    [DllImport("User32.dll", CharSet = CharSet.Auto, EntryPoint = "SendMessage")]
    static extern IntPtr SendMessage(IntPtr hWnd, uint Msg, IntPtr wParam, IntPtr lParam);

    const int WM_VSCROLL = 277;
    const int SB_BOTTOM = 7;

    private void txtOut_TextChanged(object sender, EventArgs e) {            
        IntPtr ptrWparam = new IntPtr(SB_BOTTOM);
        IntPtr ptrLparam = new IntPtr(0);
        SendMessage(((RichTextBox)sender).Handle, WM_VSCROLL, ptrWparam, ptrLparam); 
    }
    #endregion

    private void ConsoleForm_FormClosed(object sender, FormClosedEventArgs e) {
        this.stop = true;
        try {
            this.p.Kill();
        } catch (InvalidOperationException) {
            return;
        }
    }

Another interesting this is that it doesn't always hide the cmd window like it's supposed to. It hides it the first time, and then the second (or after) it won't hide it. This is when the user can close the cmd.exe window and cause the readline to act funny. It also never reads the last line outputted to cmd unless it exits.

Any suggestions on how to fix this?

Answer 1

Whenever you have a while(true) loop in your code you're going to peg your cpu (or at least one core) at 100%, unless you also have a way to break out of the loop. In your case, you do have a return statement, but at no point in the loop do you ever do anything to the stop variable guarding it.

Answer 2

Having while(true) with no sleep in the loop will cause 100% CPU usage.

You need to sleep for some amount of time or break out of the loop at some point so the CPU can do something else.

At the very least you should be doing something along the lines of:

while (sr.Peek() >= 0) 
{
    Console.WriteLine(sr.ReadLine());
    Thread.Sleep(0);
}

Answer 3

I would change:

while(true)

to:

while(!sr.EOS) {

}

It is a better way to check to end the loop.

Answer 4

This seems like an interesting issue here. At first glance, it would appear that ReadLine has an issue with the handle being closed from under it while it's trying to read data, and thus would seem to be a bug in the Framework. However, I'm not convinced that easily that it's a bug in the .Net framework...

However, there are a couple low level issues here.

The other answers you have got so far all suggest you modify the while loop. I would do this as well, but I don't think this is the root of your problem. You do not need a sleep in there, because you will get your waitstate from the ReadLine(), unless there is no data to read, and it just returns a failue, THEN you will 'tight-loop'. So, make sure you are checking any and all error states during this loop.

If you do not, I can see issues.

If everything else is working as it should, then if I were you, I would start by trying to identify if you can duplicate it outside of your program with a small demo program. I'm sure there is plenty of error checking in the Framework's Stream handling. However, it looks like you are running some stuff from Cygwin, and that's the output you are reading from the cmd shell.

Try making a simple app that just spits out data to stdout, and stderr, and then make sure the app closes while you are still reading..

Also use the debugger to see what line== after the failure occurs.

Larry

Answer 5

@Rich, No, this loop he has will NOT tightloop and cause 100% CPU usage because ReadLine is a blocking function. Trust me. I've had this argument on another topic on here already. As mentioned, he found his issue by the outer loop never exiting, (and in this case, yes using 100%, because Readline fails, leaving him with no wait-state.

But again, that was NOT the cause of his problem, period.