Question

I sometimes encounter code in the following form:

while (true) {
  //do something
  Thread.Sleep(1000);
}

I was wondering if this is considered good or bad practice and if there are any alternatives.

Usually I "find" such code in the main-function of services.

I recently saw code in the "Run" function in a windows azure worker role which had the following form:

ClassXYZ xyz = new ClassXYZ(); //ClassXYZ creates separate Threads which execute code
while (true) {
  Thread.Sleep(1000);
}

I assume there are better ways to prevent a service (or azure worker role) from exiting. Does anyone have a suggestion for me?

Was it helpful?

Solution

Well when you do that with Thread.Sleep(1000), your processor wastes a tiny amount of time to wake up and do nothing.

You could do something similar with CancelationTokenSource.

When you call WaitOne(), it will wait until it receives a signal.

CancellationTokenSource cancelSource = new CancellationTokenSource();

public override void Run()
{
    //do stuff
    cancelSource.Token.WaitHandle.WaitOne();
}

public override void OnStop()
{
    cancelSource.Cancel();
}

This will keep the Run() method from exiting without wasting your CPU time on busy waiting.

OTHER TIPS

An alternative approach may be using an AutoResetEvent and instantiate it signaled by default.

public class Program
{
     public static readonly AutoResetEvent ResetEvent = new AutoResetEvent(true);

     public static void Main(string[] args) 
     {
          Task.Factory.StartNew
          (
               () => 
               {
                   // Imagine sleep is a long task which ends in 10 seconds
                   Thread.Sleep(10000);

                   // We release the whole AutoResetEvent
                   ResetEvent.Set();
               }
          );

          // Once other thread sets the AutoResetEvent, the program ends
          ResetEvent.WaitOne();
     }
}

Is the so-called while(true) a bad practice?

Well, in fact, a literal true as while loop condition may be considered a bad practice, since it's an unbrekeable loop: I would always use a variable condition which may result in true or false.

When I would use a while loop or something like the AutoResetEvent approach?

When to use while loop...

...when you need to execute code while waiting the program to end.

When to use AutoResetEvent approach...

...when you just need to hold the main thread in order to prevent the program to end, but such main thread just needs to wait until some other thread requests a program exit.

If you see code like this...

while (true)
{
  //do something
  Thread.Sleep(1000);
}

It's most likely using Sleep() as a means of waiting for some event to occur — something like user input/interaction, a change in the file system (such as a file being created or modified in a folder, network or device event, etc. That would suggest using more appropriate tools:

  • If the code is waiting for a change in the file system, use a FileSystemWatcher.
  • If the code is waiting for a thread or process to complete, or a network event to occur, use the appropriate synchronization primitive and WaitOne(), WaitAny() or WaitAll() as appropriate. If you use an overload with a timeout in a loop, it gives you cancelability as well.

But without knowing the actual context, it's rather hard to say categorically that it's either good, bad or indifferent. If you've got a daemon running that has to poll on a regular basis (say an NTP client), a loop like that would make perfect sense (though the daemon would need some logic to monitor for shutdown events occuring.) And even with something like that, you could replace it with a scheduled task: a different, but not necessarily better, design.

If you use while(true) you have no programmatic means of ending the loop from outside the loop.

I'd prefer, at least, a while(mySingletonValue) which would allow us to switch the loop as needed.

An additional approach would be to remove the functional behavior from the looping behavior. Your loop my still be infinite but it calls a function defined elsewhere. Therefore the looping behavior is completely isolated to what is being executed by the loop:

while(GetMySingletonValue())
{
    someFunction();
}

In this way your singleton controls the looping behavior entirely.

It really depends on that //do something on how it determines when to break out of the loop.

In general terms, more appropriate way to do it is to use some synchronization primitive (like ManualResetEvent) to wait on, and the code that processes and triggers the break of the loop (on the other thread) to signal on that primitive. This way you don't have thread wasting resources by being scheduled in every second to do nothing, and is a much cleaner way to do it.

I personally don't like Thread.Sleep code. Because it locks the main thread. You can write something like this, if it is a windows application besides it allows you more flexibility and you can call it async:

bool switchControl = true;

while (switchControl) {
  //do something
  await Wait(1);
}

async void Wait(int Seconds)
{
  DateTime Tthen = DateTime.Now;
  do
  {
    Application.DoEvents(); //Or something else or leave empty;
  } while (Tthen.AddSeconds(Seconds) > DateTime.Now);
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top