Reputation: 7489
I want to run a function on an interval, inside of a Task. Something like,
Task t = Task.Factory.StartNew(() => {
while (notCanceled()) {
doSomething();
Thread.Sleep(interval);
}
});
Is it a bad idea to use Thread.Sleep() here? The task is long running, and the sleep time may also be very long (minutes, maybe even hours).
One alternative is to use either System.Timers.Timer or System.Threading.Timer. However both of these would cause an additional thread to spawn (the Elapsed events occur on a new threadpool thread). So for every repeating task, there would be 2 threads instead of 1. The Task is already asynchronous so I'd prefer not to complicate things in this way.
Yet another way that behaves similarly is to use ManualResetEvent,
ManualResetEvent m = new ManualResetEvent(false);
void sleep(int milliseconds)
{
m.WaitOne(milliseconds);
}
Since m.Set() would never be called, this would always wait for the right amount of time, and also be single threaded. Does this have any significant advantage over Thread.Sleep()?
Wondering what the best practice would be here.
Thoughts?
Upvotes: 2
Views: 4217
Reputation: 134005
Yes, it is a very bad idea to use sleep in a loop like that.
You have a faulty understanding of timers. Creating a timer does not create a new thread. The timer sets an operating system trigger that, when the time elapses, spawns a threadpool thread. So if you write:
System.Threading.Timer myTimer =
new Timer(DoStuff, null,
TimeSpan.FromMinutes(10), TimeSpan.FromMinutes(10));
The only time there will be another thread is when the handler method (DoStuff
) is executing.
There'd be no reason to have the task if everything it does is handled by your DoStuff
method. If you want to cancel it, just dispose the timer.
I strongly recommend, by the way, that you not use System.Timers.Timer
. In short, it squashes exceptions, which hides bugs. See Swallowing exceptions is hiding bugs.
Upvotes: 3
Reputation: 203828
If you're using C# 5.0 you can use:
while(notCanceled())
{
doSomething();
await Task.Delay(interval);
}
If you're using an earlier version your best bet is probably to use a Timer
.
Both of the code samples you showed, involving either Thread.Sleep
or a ManualResetEvent
are blocking the current thread for that duration, which means your code is tying up a thread which can't do anything else until your task is canceled. You don't want to do that. If you use a timer, or the await
code mentioned above, you will end up not blocking any thread at all while waiting, and then use up a thread pool's time only when you have productive work to be doing.
Upvotes: 5