0

I have this method ExecuteAllAsync() which uses the field List jobs and executes asynchronously all Jobs in the list . I want to set a timeout to kill the job and stop the execution if it takes to long or no longer necessary. I tried doing so in the following code:


        //Execute all jobs async
        public async void ExecuteAllAsync()
        {
            if (jobs.Count()==0)
                return;
            List<Task> tasks = new List<Task>();
            foreach (Job job in jobs.ToArray())
            {
                int timeout = 500; 

                Task t = Task.Run(() => { 
                     ExecuteJob(job) // Do Work Here
                });
                tasks.Add(t);
                if (!t.Wait(timeout))
                     //Log Timeout Error
            }
            await Task.WhenAll(tasks);
        }

This doesn't work. I tried to also put Thread.Sleep(1000) in the ExecuteJob() Function to make all tasks timeout and and the result is that only some of the jobs timeout and some execute fully. I also use stopwatch to check how long the job takes and I see some of them takes more than 1 second.

Currently the timeout is set 500 ms but in the future each task will have different timeout value:

int timeout = (int)(job.TimeToKill() - DateTime.Now).TotalMilliseconds

How do I execute all jobs in list asynchronously while setting a timeout to each task? Thanks in advance.

Ron D
  • 23
  • 2
  • 6
  • 1
    Possible duplicate of [this](https://stackoverflow.com/questions/35645899/awaiting-task-with-timeout) and [this](https://stackoverflow.com/questions/4238345/asynchronously-wait-for-taskt-to-complete-with-timeout) – aepot May 17 '20 at 11:36
  • What do you mean? – Ron D May 17 '20 at 11:39
  • I mean that is possible duplicate. And it doesn't clear where timeout must be set? For all tasks or for each separate `Task`? For example you set timeout 1s for each one and all jobs start will performed in 500ms. Then you'll get finish in ~1,5s. Or you want to set timeout for all jobs execution and then teminate all not finished in time? BTW, `t.Wait()` in `async` method is mistake and may cause locking of the main UI Thread while waiting. – aepot May 17 '20 at 11:47
  • 1
    Does the `ExecuteJob` method accept a [`CancellationToken`](https://learn.microsoft.com/en-us/dotnet/standard/threading/cancellation-in-managed-threads) as argument? If not, there is no clean way to kill the task. You may think about `Thread.Abort`, but this is a [no-no](https://stackoverflow.com/questions/1559255/whats-wrong-with-using-thread-abort), and also not supported in .NET Core. – Theodor Zoulias May 17 '20 at 17:49
  • @RonD did not answers satisfy your question? else please accept one. – sommmen Sep 16 '20 at 13:13

2 Answers2

4

You could use a CancellationToken:

You can create one with a CancellationTokenSource and then pass a timeout to .CancelAfter(int);

https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtokensource.cancelafter?view=netcore-3.1

Make sure to check the cancellationToken inside your task.

sommmen
  • 6,570
  • 2
  • 30
  • 51
0

You can use this simple approach:

public void ExecuteAll() //Note: since ExecuteJob is not async, this method not needs to be async anymore
{
    if (jobs.Count() == 0)
        return;

    int timeout = 500; 

    Parallel.ForEach(jobs, job => ExecuteJob(job, timeout);
}


public void ExecuteJob(Job job, int tiemout) //Your new ExecuteJob overload receiving a Timeout and using a CancellationToken internally
{
    using (var cts = new CancellationTokenSource(timeout))
    {
        foreach (var jobItem in job.ItemsToProcess) //Assuming you have something to do inside our job
        {
            if (cts.Token.IsCancellationRequested)
            {
                // ==> Log Timeout Error; cancel your job 
            }

            // Do your job
        }
    }
}

Note you can simply use Parallel.ForEach, it would take care for you about the multiple threads, optimizing them according to the numbers of cores available in the executing machine

CicheR
  • 359
  • 1
  • 12
  • 1
    This code creates a memory leak. On each call of `ExecuteAll` you create a new instance of `CancellationTokenSource` which does not get disposed and so may or may not release its unmananged resources. Please be aware that many people just copy code of stackoverflow. You just need to wrap the call to `Parallel.ForEach` in a using block with `using (var cts = new CancellationTokenSource(timeout)) { ... }` – Ackdari May 18 '20 at 11:36
  • Answer updated to using `using` for the tokens. Thanks @Ackdari for the advice – CicheR May 20 '20 at 00:46