6

I have recently seen several recommendations stating that Thread.Sleep should never be used in production code (most recently in this SO question). Many of these advocate for using Task.Delay instead. Most of the explanations I've found use UI applications as examples, since the advantages to Task.Delay are obvious (not blocking the UI).

In my case, I am using Thread.Sleep inside of a wait loop that polls a WCF service for a particular condition, like this:

DateTime end = DateTime.UtcNow + TimeSpan.FromMinutes(2);
while (DateTime.UtcNow < end)
{
    if (ExternalServiceIsReady() == true)
    {
        return true;
    }
    Thread.Sleep(1000);
}

In this case, the following potential advantages of Task.Delay seem not to apply:

  • The sleep time is fairly large relative to the typical timer resolution of around 15 ms, so the increase in accuracy of Task.Delay seems trivial.
  • The process is single-threaded (non-UI) and must block until the condition is true, so using await has no advantage here.
  • The ability to cancel the delay is not required.

Is this a case where it is appropriate to use Thread.Sleep? What would be the advantage (if any) of replacing my sleep line with Task.Delay(1000).Wait()?

Community
  • 1
  • 1
BJ Myers
  • 6,617
  • 6
  • 34
  • 50
  • 2
    What does `ExternalServiceIsReady` do ? Maybe Sleep/Delay can be completely avoided. – EZI Mar 30 '15 at 20:59
  • 2
    Is there an option C to use an event? It might be more appropriate than this polling approach. – Jeroen Vannevel Mar 30 '15 at 21:01
  • 1
    Option D is the superior solution, a Timer. – Hans Passant Mar 30 '15 at 22:06
  • 1
    @HansPassant Can you elaborate on why a Timer is a superior solution? – BJ Myers Mar 30 '15 at 22:18
  • 1
    No loop, no thread, no sleep. – Hans Passant Mar 30 '15 at 22:26
  • Aside - it would be better to use a `System.Diagnostics.Stopwatch` instead of checking `DateTime.UtcNow`. – Matt Johnson-Pint Mar 30 '15 at 22:32
  • @HansPassant That would make it better than `Thread.Sleep`, but still doesn't seem to provide any advantage over `Task.Delay`. Wouldn't I still need a loop to keep calling the polling function? – BJ Myers Mar 30 '15 at 23:15
  • Hard to see why you bring this up, you were already determined to not use it. – Hans Passant Mar 30 '15 at 23:17
  • @HansPassant You've said that a Timer is a superior solution, and I can see how avoiding a sleeping thread is advantageous, but I don't understand why else the Timer is a superior solution? Why would I use a Timer when either Thread.Sleep or Task.Delay are so much simpler? I don't see how a Timer helps me get rid of the loop. – BJ Myers Mar 30 '15 at 23:24
  • @MattJohnson I've heard that before too, but the only advantage I've heard for Stopwatch is better accuracy, which I really don't need. Are there advantages to using Stopwatch when the times measured are on the order of seconds, not milliseconds? – BJ Myers Mar 30 '15 at 23:27
  • 1
    @nintendojunkie - Consider that clock can be *modified* (by the user, by NTP, etc.) – Matt Johnson-Pint Mar 31 '15 at 00:07

1 Answers1

7

There's never an advantage in replacing Thread.Sleep(1000); in Task.Delay(1000).Wait();. If you want to wait synchronously just use Thread.Sleep.

If you really only have a single thread and planning to keep it that way, then you can use Thread.Sleep. However, I would still use Task.Delay as it's preferable in most cases and so it's a good pattern. I would only block at very top when you can't use async anymore, and even then I would suggest using some kind of AsyncContext.

You can also use a System.Threading.Timer directly* instead of Task.Delay however you should keep in mind that the timer executes every interval and doesn't wait for the actual operation to complete, so if ExternalServiceIsReady takes more than the interval you can have multiple calls to that service concurrently.

An even better solution would be to replace the polling of the external service with an asynchronous operation so the service can notify you when it's ready instead of you asking it every second (that isn't always possible as it depends on the service):

await ExternalServiceIsReadyAsync();

* Task.Delay uses a System.Threading.Timer internally which also has a resolution of ~15ms.

i3arnon
  • 113,022
  • 33
  • 324
  • 344
  • 2
    And a more better way would be if `ExternalServiceIsReady` can throw an event that way there would be no need for sleep/wait. – EZI Mar 30 '15 at 22:07
  • @EZI That's asynchronous. – i3arnon Mar 30 '15 at 22:08
  • And what? doesn't it loop and sleep? What does asynchronous call change here? You only involve another task here. – EZI Mar 30 '15 at 22:09
  • @EZI An asynchronous operation means that it notifies you when it completes. It doesn't matter if it's done by competing a `Task`, invoking a callback, or raising an event. It's all the same and can be converted to other methods. – i3arnon Mar 30 '15 at 22:11
  • @ i3arnon I know what an async operation is, but it seems you don't understand its usage. Replacing that line with `await ExternalServiceIsReadyAsync();` wouldn't change anything. – EZI Mar 30 '15 at 22:14
  • @EZI Replacing a synchronous blocking loop with an asynchronous awaitable call would mean one less thread (and its CPU cycles) wasted. – i3arnon Mar 30 '15 at 22:17
  • 1
    @i3arnon For your last comment: `Thread.Sleep` doesn't waste any cpu cycle. It keeps a usable resource(thread) hanging around, but no cpu cycles spent. Do you mean that while loop and those conditions? – Sriram Sakthivel Mar 31 '15 at 18:16
  • @SriramSakthivel `Thread.Sleep` (as do `Task.Delay`) doesn't waste CPU cycles. It's the constant polling that does, and a single asynchronous operation (be it `async` or otherwise) solves that. – i3arnon Mar 31 '15 at 18:38
  • @i3arnon To be clear - In a constant polling still `Thread.Sleep` doesn't wastes cpu cycles; the code which does the polling does. – Sriram Sakthivel Mar 31 '15 at 18:41
  • @SriramSakthivel of course. Just as `Task.Delay` doesn't. (though if we want to nitpick, `Thread.Sleep` would cause an unneeded context switch that *can* waste CPU cycles) – i3arnon Mar 31 '15 at 18:45
  • 2
    Another nitpick. `Task.Delay` creates a `Task`(memory allocation), `task.Wait()` will spin for certain amount of time and `task.Wait()` will allocate a Kernel event after that point (kernel resource and user-> kernel transition) all of which is gonna take cpu cycles :) – Sriram Sakthivel Mar 31 '15 at 18:48