Replacing Thread.Sleep with Task.Delay

Question

I am refactoring code that uses Thread.Sleep with an increasing time limit to retry SQL queries when there were errors. The common replacement for Thread.Sleep for blocking seems to be await Task.Delay, which required changing the method to async. The method now looks like this (additional error checking removed for brevity):

    private static async Task<int> PrepareForRetry( int retryCount, SqlCommand command, int timeout )
    {
        ++retryCount;
        if (retryCount < ConnectionRetryCount)
        {
            int SleepTime = _connectionRetryBackoffTimes[retryCount - 1] + (_connectionRetryRandomBackoff.Next() % 500);
            //Thread.Sleep(SleepTime);
            await Task.Delay(SleepTime);
        }
        return retryCount;
    }

The problem that I'm having is that async requires the calling method to be async, and so forth. Although eventually refactoring to fully asynchronous, this is well beyond the current refactor scope.

The issue I'm having is how to call the method from synchronous code and getting the result. Using

retryCount = PrepareForRetry(retryCount, command, timeout).Result;

creates a deadlock, since a UI thread is calling the method. I've seen that I can resolve this by changing my Task.Delay to

await Task.Delay(SleepTime).ConfigureAwait(false);

but I don't fully understand what this does. I've also tried calling the method using

retryCount = Task.Run(async () => { await PrepareForRetry(retryCount, command, timeout).Result; });

but this has the error " 'int' does not contain a definition for 'GetAwaiter' ", and I've not been able to find out how I can proceed to get the result. Is using Task.Delay the correct way of creating the delay (a timer doesn't allow for the increase in wait times), and if so, how should I call the method to get the return value?

Side note: why are you doing that? Unless you go all out and make everything properly async there is no upside for local UI apps (see https://stackoverflow.com/questions/20082221/when-to-use-task-delay-when-to-use-thread-sleep?rq=1, which may be even duplicate depending on your actual goal). Side note 2: I'd start conversion from the top and not the bottom - it is ok to call synchronous method from `async` one, but really not the other way around. — Alexei Levenkov, Jul 02 '19 at 01:15
The rule of thumb is if you need to call `.Result` on an `async` method or you are offloading it to a new *thread* with `Task.Run` you are more than likely doing something wrong, in short, use `Thread.Sleep` unless you want to propagate the entire call stack to `async` — TheGeneral, Jul 02 '19 at 01:48
I don’t understand the need for a delay. Are you trying to cope with timeout issues on your database connection? There are better ways to handle that. — John Wu, Jul 02 '19 at 03:00
@JohnWu - I would love to handle this the correct way. This is the root of our DAL, and obviously if database access fails our program doesn't work. I'm trying to find the proper way to handle SqlException to give the highest likelihood for data to be retrieved or saved, but am not sure where to start. Can you suggest an article or pattern name to use? — Tim, Jul 02 '19 at 16:33
I still don't understand the need for an increasing delay. Are your queries timing out? Is there network intermittancy? The "correct" way of course is to fix the underlying problem, but if you need a band-aid, the nature of the specific problem is important. — John Wu, Jul 02 '19 at 20:21
There are some queries on larger databases that I have been told can take up to 10 minutes to complete. During this time, the database is locked. The query needs to retry once the database is unlocked again. The software is installed on several workstations, so the database could be accessed by multiple requests at a time. — Tim, Jul 02 '19 at 23:25

score 5 · Answer 1 · answered Jul 02 '19 at 02:37

5

You should use the same style for delays as you do for the actual operation. If your SqlCommand is executed asynchronously, then use Task.Delay for delays. If your SqlCommand is executed synchronously, then use Thread.Sleep for delays.

It sounds like you still need your SqlCommand to be executed synchronously (at least for now), so you should be using Thread.Sleep. Eventually, you could make them both asynchronous (at the same time), but it sounds like that is work for another day.

It's not true that Task.Delay is somehow a replacement for Thread.Sleep. It's just that Task.Delay is the asynchronous equivalent of Thread.Sleep.

answered Jul 02 '19 at 02:37

Stephen Cleary

437,863
77
675
810

1

Thank you for the clarification. We had a code review done, and were told we should remove all Thread.Sleeps, so I was tasked with doing this. Many of them I could convert to timers, but this one seems to require a much deeper refactoring. – Tim Jul 02 '19 at 16:39
1

Removing `Thread.Sleep` here is good in theory, but it should be done in conjunction with making `SqlCommand` async, and spreading that async up and elsewhere in your codebase. I wouldn't say "`Thread.Sleep` is bad" as a blanket statement; in this case it's OK (assuming you're not ready for this async refactoring). If you want to abide by the letter of the law and not the spirit, then you can use [Polly](https://github.com/App-vNext/Polly) for retry logic, which uses an equivalent of `Thread.Sleep` internally for its synchronous retries. – Stephen Cleary Jul 02 '19 at 16:53

Replacing Thread.Sleep with Task.Delay

1 Answers1