Why using System.Threading.Thread.Sleep() is a bad

Question

I'm writing an application working with a big and ugly 3rd party system via a complicated API. Sometimes some errors happen in the system, but if we wait for my program to face this errors it can be too late.

So, I use a separate thread to check the system state as following:

while (true)
{
    ask_state();
    check_state();
    System.Threading.Thread.Sleep(TimeSpan.FromSeconds(1));
}

It doesn't really matter if I check the system state once in 100 ms or once a minute.

But I have heard that using Thread.Sleep() is a bad practice. Why? And what can I do in this situation?

Answer 1

Because the only way to shut down this thread if it's waiting inside the Sleep is to either a) wait for the Sleep to end, or b) use one of Thread.Abort or Thread.Interrupt.¹

If it's a long sleep, then (a) isn't really suitable if you're trying to be responsive. And (b) are pretty obnoxious if the code happens to not actually be inside the Sleep at the time.

It's far better, if you want to be able to interrupt the sleeping behaviour in a suitable fashion, to use a waitable object (such as e.g. a ManualResetEvent) - you might then even be able to place the wait on the waitable object into the while conditional, to make it clear what will cause the thread to exit.

---

¹ I've use shutdown in this instance because it's a very common scenario where cross-thread communication is required. But for any other cross-thread signalling or communication, the same arguments can also apply, and if it's not shutdown then Thread.Abort or Thread.Interrupt are even less suitable.

Answer 2

i would set a timer to whatever ms you want and wait for my check methods to complete, by the way do you want to use an eternal loop or it is not a complete code that you showed up there ?
ok this is a sample of what i'm talking about:

public void myFunction()
{
int startCount = Environment.TickCount;
ask_state();
check_state();

while (true)
{
if (Environment.TickCount - startCount >= 20000) //two seconds 
{
break;
}
Application.DoEvents();
}
}

//Now you have an organized function that makes the task you want just call it every
// time interval, again you can use a timer to do that for you

private void timer_Tick(object sender, EventArgs e)
{
            myFunction();
}

good luck

Answer 3

One reason is that Thread.Sleep() is blocking your code from doing anything else. Recent efforts is to make blocking as least as possible. For example, node.js is a non-blocking language.

Update: I don't know about the infrastructure of Timer class in C#. Maybe it's also blocking.

You can schedule a task to check that third API every 100 ms. This way, during that 100 ms, your program can do other tasks.

Update: This analogy might help. If we compare operating system to a hospital, and compare the threads to nurses in that hospital, the supervisor (programmer) can choose a policy:

1. Either to ask each nurse (thread) to watch one, and only one patient (a job, a task to be done), even if between each check she waits for an hour (Sleep() method)
2. To ask each nurse to check each patient, and during the interval till next check, go on and check other patients.

The first model is blocking. It's not scalable. But in the second model, even with few nurses, you might be able to serve many patients.