views:

183

answers:

6

Consider a Console application that starts up some services in a separate thread. All it needs to do is wait for the user to press Ctrl+C to shut it down.

Which of the following is the better way to do this?

static ManualResetEvent _quitEvent = new ManualResetEvent(false);

static void Main() {
    Console.CancelKeyPress += (sender, eArgs) => {
        _quitEvent.Set();
        eArgs.Cancel = true;
    };

    // kick off asynchronous stuff 

    _quitEvent.WaitOne();

    // cleanup/shutdown and quit
}

Or this, using Thread.Sleep(1):

static bool _quitFlag = false;

static void Main() {
    Console.CancelKeyPress += delegate {
        _quitFlag = true;
    };

    // kick off asynchronous stuff 

    while (!_quitFlag) {
        Thread.Sleep(1);
    }

    // cleanup/shutdown and quit
}
+6  A: 

you always want to prevent using while loops, especially when you are forcing the code to recheck variables. It wastes CPU resources and slows down your program.

I would definitely say the first one.

Mike
+1. In addition, since the `bool` isn't declared as `volatile`, there is the definite possibility that subsequent reads to `_quitFlag` in the `while` loop would be optimized away, leading to an infinite loop.
Adam Robinson
+7  A: 

Alternatively, a more simple solution is just:

Console.ReadLine();
Cocowalla
I was about to suggest that, but it won't stop only on Ctrl-C
Thomas Levesque
I got the impression that CTRL-C was just an example - any user input to close it
Cocowalla
+2  A: 

Of the two first one is better

_quitEvent.WaitOne();

because in the second one the thread wakes up every one millisecond will get turned in to OS interrupt which is expensive

Naveen
+5  A: 

You can do that (and remove the CancelKeyPress event handler) :

while(!_quitFlag)
{
    var keyInfo = Console.ReadKey();
    _quitFlag = keyInfo.Key == ConsoleKey.C
             && keyInfo.Modifiers == ConsoleModifiers.Control;
}

Not sure if that's better, but I don't like the idea of calling Thread.Sleep in a loop.. I think it's cleaner to block on user input.

Thomas Levesque
Damn. I halfway typed up that exact example and then it says "New Answers!". +1
Chris T
A: 

You should do it just like you would if you were programming a windows service. You would never use a while statement instead you would use a delegate. WaitOne() is generally used while waiting for threads to dispose - Thread.Sleep() - is not advisible - Have you thought of using System.Timers.Timer using that event to check for shut down event?

KenL
A: 

Seems like you're making it harder than you need to. Why not just Join the thread after you've signaled it to stop?

class Program
{
    static void Main(string[] args)
    {
        Worker worker = new Worker();
        Thread t = new Thread(worker.DoWork);
        t.IsBackground = true;
        t.Start();

        while (true)
        {
            var keyInfo = Console.ReadKey();
            if (keyInfo.Key == ConsoleKey.C && keyInfo.Modifiers == ConsoleModifiers.Control)
            {
                worker.KeepGoing = false;
                break;
            }
        }
        t.Join();
    }
}

class Worker
{
    public bool KeepGoing { get; set; }

    public Worker()
    {
        KeepGoing = true;
    }

    public void DoWork()
    {
        while (KeepGoing)
        {
            Console.WriteLine("Ding");
            Thread.Sleep(200);
        }
    }
}
ebpower
In my case I don't control the threads that the asynchronous stuff runs on.