0

I am using Async await with Task.Factory method.

public async Task<JobDto> ProcessJob(JobDto jobTask)
{
    try
    {
        var T = Task.Factory.StartNew(() =>
        {
            JobWorker jobWorker = new JobWorker();
            jobWorker.Execute(jobTask);
        });

        await T;
    }

This method I am calling inside a loop like this

for(int i=0; i < jobList.Count(); i++)
{
    tasks[i] = ProcessJob(jobList[i]);
}

What I notice is that new tasks opens up inside Process explorer and they also start working (based on log file). however out of 10 sometimes 8 or sometimes 7 finishes. Rest of them just never come back.

  1. why would that be happening ?
  2. Are they timing out ? Where can I set timeout for my tasks ?

UPDATE

Basically above, I would like each Task to start running as soon as they are called and wait for the response on AWAIT T keyword. I am assuming here that once they finish each of them will come back at Await T and do the next action. I am alraedy seeing this result for 7 out of 10 tasks but 3 of them are not coming back.

Thanks

Zeus
  • 3,091
  • 6
  • 47
  • 60
  • how are you waiting for them? Are there any exceptions thrown? – NeddySpaghetti Oct 01 '15 at 12:22
  • 4
    Did you try `Task.WaitAll(tasks)` after your for loop? You are not waiting for your `tasks` to complete. – Arghya C Oct 01 '15 at 12:23
  • @Ned I am waiting by using await key word.... – Zeus Oct 01 '15 at 19:49
  • @ArghyaC I do not want to do WaitAll because than I will have to wait for all the tasks to finish before doing next action. I do not want ot block the call....basically I want each task to start running and do their thing right away. Once they are done than come back and report it to me on Await T word – Zeus Oct 01 '15 at 19:51
  • 1
    You really [should not](http://blog.stephencleary.com/2013/08/startnew-is-dangerous.html) be combining `await` with `TaskFactory.StartNew`, use `Task.Run(` instead. – Scott Chamberlain Oct 01 '15 at 19:55
  • Do you mean use await with Task.Run...I tried that and had similar result. – Zeus Oct 01 '15 at 19:59
  • @Zeus I've added an answer with explanation, see if that helps. – Arghya C Oct 02 '15 at 07:25
  • My problem is closure variable i which is changing inside the loop...Jon skeet explained it here http://stackoverflow.com/questions/271440/captured-variable-in-a-loop-in-c-sharp – Zeus Oct 04 '15 at 21:22

3 Answers3

2

It is hard to say what the issues is without the rest if the code, but you code can be simplified by making ProcessJob synchronous and then calling Task.Run with it.

public JobDto ProcessJob(JobDto jobTask)
{
    JobWorker jobWorker = new JobWorker();
    return jobWorker.Execute(jobTask);
}

Start tasks and wait for all tasks to finish. Prefer using Task.Run rather than Task.Factory.StartNew as it provides more favourable defaults for pushing work to the background. See here.

for(int i=0; i < jobList.Count(); i++)
{
    tasks[i] = Task.Run(() => ProcessJob(jobList[i]));
}

try
{
   await Task.WhenAll(tasks);
}
catch(Exception ex)
{
   // handle exception
}
NeddySpaghetti
  • 13,187
  • 5
  • 32
  • 61
  • I do not want to block the call so I am not doing WhenAll...I am basically starting the Task but Task.Run and doing await – Zeus Oct 01 '15 at 19:47
2

First, let's make a reproducible version of your code. This is NOT the best way to achieve what you are doing, but to show you what is happening in your code!

I'll keep the code almost same as your code, except I'll use simple int rather than your JobDto and on completion of the job Execute() I'll write in a file that we can verify later. Here's the code

public class SomeMainClass
{
    public void StartProcessing()
    {
        var jobList = Enumerable.Range(1, 10).ToArray();
        var tasks = new Task[10];
        //[1] start 10 jobs, one-by-one
        for (int i = 0; i < jobList.Count(); i++)
        {
            tasks[i] = ProcessJob(jobList[i]);
        }
        //[4] here we have 10 awaitable Task in tasks
        //[5] do all other unrelated operations
        Thread.Sleep(1500); //assume it works for 1.5 sec
        // Task.WaitAll(tasks); //[6] wait for tasks to complete
        // The PROCESS IS COMPLETE here
    }

    public async Task ProcessJob(int jobTask)
    {
        try
        {
            //[2] start job in a ThreadPool, Background thread
            var T = Task.Factory.StartNew(() =>
            {
                JobWorker jobWorker = new JobWorker();
                jobWorker.Execute(jobTask);
            });
            //[3] await here will keep context of calling thread
            await T; //... and release the calling thread
        }
        catch (Exception) { /*handle*/ }
    }
}

public class JobWorker
{
    static object locker = new object();
    const string _file = @"C:\YourDirectory\out.txt";
    public void Execute(int jobTask) //on complete, writes in file
    {
        Thread.Sleep(500); //let's assume does something for 0.5 sec
        lock(locker)
        {
            File.AppendAllText(_file, 
                Environment.NewLine + "Writing the value-" + jobTask);
        }
    }
}

After running just the StartProcessing(), this is what I get in the file

Writing the value-4
Writing the value-2
Writing the value-3
Writing the value-1
Writing the value-6
Writing the value-7
Writing the value-8
Writing the value-5

So, 8/10 jobs has completed. Obviously, every time you run this, the number and order might change. But, the point is, all the jobs did not complete!

Now, if I un-comment the step [6] Task.WaitAll(tasks);, this is what I get in my file

Writing the value-2
Writing the value-3
Writing the value-4
Writing the value-1
Writing the value-5
Writing the value-7
Writing the value-8
Writing the value-6
Writing the value-9
Writing the value-10

So, all my jobs completed here!

Why the code is behaving like this, is already explained in the code-comments. The main thing to note is, your tasks run in ThreadPool based Background threads. So, if you do not wait for them, they will be killed when the MAIN process ends and the main thread exits!!

If you still don't want to await the tasks there, you can return the list of tasks from this first method and await the tasks at the very end of the process, something like this

public Task[] StartProcessing()
{
    ...
    for (int i = 0; i < jobList.Count(); i++)
    {
        tasks[i] = ProcessJob(jobList[i]);
    }
    ...
    return tasks;
}

//in the MAIN METHOD of your application/process
var tasks = new SomeMainClass().StartProcessing();
// do all other stuffs here, and just at the end of process
Task.WaitAll(tasks);

Hope this clears all confusion.

Arghya C
  • 9,805
  • 2
  • 47
  • 66
  • Hi, Great explaination....my problem is if I do Task.Waitall than my loop will have to wait. This entire code is running inside windows service with timer so I do not want to block for all tasks to finish. This could be a problem in my design. Thoughts ?? – Zeus Oct 02 '15 at 07:54
  • ....looks like I am on a timer event so it recycles the old thread...this will cause my await tasks to get lost...correct ? – Zeus Oct 02 '15 at 07:56
  • It'd be great if @StephenCleary could help here with better approach and/or explanation :) – Arghya C Oct 02 '15 at 08:05
  • @Zeus It's hard to comment without seeing your actual code, but the Tasks should complete given the calling thread doesn't exit or enough time. Check if it's possible to do something in the line of the alternative code given at the end. – Arghya C Oct 02 '15 at 08:08
1

It's possible your code is swallowing exceptions. I would add a ContineWith call to the end of the part of the code that starts the new task. Something like this untested code:

var T = Task.Factory.StartNew(() =>
        {
            JobWorker jobWorker = new JobWorker();
            jobWorker.Execute(jobTask);
        }).ContinueWith(tsk =>
        {
            var flattenedException = tsk.Exception.Flatten();
            Console.Log("Exception! " + flattenedException);
            return true;
         });
        },TaskContinuationOptions.OnlyOnFaulted);  //Only call if task is faulted

Another possibility is that something in one of the tasks is timing out (like you mentioned) or deadlocking. To track down whether a timeout (or maybe deadlock) is the root cause, you could add some timeout logic (as described in this SO answer):

int timeout = 1000; //set to something much greater than the time it should take your task to complete (at least for testing)
var task = TheMethodWhichWrapsYourAsyncLogic(cancellationToken);
if (await Task.WhenAny(task, Task.Delay(timeout, cancellationToken)) == task)
{
    // Task completed within timeout.
    // Consider that the task may have faulted or been canceled.
    // We re-await the task so that any exceptions/cancellation is rethrown.
    await task;

}
else
{
    // timeout/cancellation logic
}

Check out the documentation on exception handling in the TPL on MSDN.

Community
  • 1
  • 1
Noel
  • 3,288
  • 1
  • 23
  • 42