Is it always bad to use Thread.Sleep()?

Is using Thread.Sleep bad? Generally not, if you really want to suspend the thread. But in this case you don't want to suspend the thread, you want to suspend the task.

So in this case, you should use:

await Task.Delay(minDuration);

This will not suspend the entire thread, but just the single task you want to suspend. All other tasks on the same thread can continue running.

One reason I would use Task.Delay over Thread.Sleep is the fact that you can pass a CancellationToken to it. If the user wants to StopExecutor and the random received a long duration span, you'll end up blocking for a long time. On the other hand, in Task.Delay, you can cancel the operation and it will be notified of that cancellation.

I think there are other problems with the design you choose to make. The Random class isn't really fit to be a task scheduler. I would find it a bit odd to find a ExecuteRandomAsync, since it mostly doesn't execute a random, but execute some arbitrary Action every X minutes.

I would instead, go about this another way. While keeping most of the internals you've already created, but putting them in a different class.

public class ActionInvoker{    private readonly Action _actionToInvoke;    public ActionInvoker(Action actionToInvoke)    {        _actionToInvoke = actionToInvoke;        _cancellationTokenSource = new CancellationTokenSource();    }    private readonly CancellationTokenSource _cancellationTokenSource;    private Task _executer;    public void Start(int min, int max, int minDuration)    {        if (_executer != null)        {            return;        }        _executer = Task.Factory.StartNew(                    async () => await ExecuteRandomAsync(min, max, _actionToInvoke),                    _cancellationTokenSource.Token, TaskCreationOptions.LongRunning,                     TaskScheduler.Default)                    .Unwrap();    }    private void Stop()    {        try        {            _cancellationTokenSource.Cancel();        }        catch (OperationCanceledException e)        {            // Log the cancellation.        }    }    private async Task ExecuteRandomAsync(int min, int max, Action action)    {        Random random = new Random();        while (!_cancellationTokenSource.IsCancellationRequested)        {            await Task.Delay(random.Next(min, max), _cancellationTokenSource.Token);            action();        }    }}

Sleep "talks" to the Operation System that you want to suspend the thread. It's resource-intensive operation cause your thread uses RAM anyway (although it doesn't require processing time).

With thread pool, you could use thread's resources (f.e. RAM) to process some other small tasks. To do it, the Windows lets you put a thread to sleep in a special alertable state, so it may be awakened and used temporarily.

So Task.Delay let you put threads to alertable sleep, and therefore let you use resources of these thread unitl you don't need them.