Task.WaitAll not waiting for task to complete

Question

While trying to figure out the new (maybe not so new now, but new to me, anyway) Task asynchronous programming in C#, I ran into a problem that took me a bit to figure out, and I'm not sure why.

I have fixed the problem, but I am still not sure why it was a problem to begin with. I just thought I'd share my experience in case anyone out there runs into the same situation.

If any gurus would like to inform me of the cause of the problem, that'd be wonderful and much appreciated. I always like knowing just why something doesn't work!

I made a test task, as follows:

Random rng = new Random((int)DateTime.UtcNow.Ticks);
int delay = rng.Next(1500, 15000);
Task<Task<object>> testTask = Task.Factory.StartNew<Task<object>>(
    async (obj) =>
        {
            DateTime startTime = DateTime.Now;
            Console.WriteLine("{0} - Starting test task with delay of {1}ms.", DateTime.Now.ToString("h:mm:ss.ffff"), (int)obj);
            await Task.Delay((int)obj);
            Console.WriteLine("{0} - Test task finished after {1}ms.", DateTime.Now.ToString("h:mm:ss.ffff"), (DateTime.Now - startTime).TotalMilliseconds);
            return obj;
        },
        delay
    );
Task<Task<object>>[] tasks = new Task<Task<object>>[] { testTask };

Task.WaitAll(tasks);
Console.WriteLine("{0} - Finished waiting.", DateTime.Now.ToString("h:mm:ss.ffff"));

// make console stay open till user presses enter
Console.ReadLine();

And then I ran the application to see what it spat out. Here is some sample output:

6:06:15.5661 - Starting test task with delay of 3053ms.
6:06:15.5662 - Finished waiting.
6:06:18.5743 - Test task finished after 3063.235ms.

As you can see, the Task.WaitAll(tasks); statement didn't do much. It waited a grand total of 1 millisecond before continuing execution.

I have answered my own "question" below - but as I said above - if anyone more knowledgeable than I would care to explain why this doesn't work, please do! (I think it might have something to do with the execution 'stepping-out' of the method once it reaches an await operator - then stepping back in once the awaiting is done... But I am probably wrong)

Answer 1

You should avoid using Task.Factory.StartNew with async-await. You should use Task.Run instead.

An async method returns a Task<T>, an async delegate does as well. Task.Factory.StartNew also returns a Task<T>, where its result is the result of the delegate parameter. So when used together it returns a Task<Task<T>>>.

All that this Task<Task<T>> does is execute the delegate until there's a task to return, which is when the first await is reached. If you only wait for that task to complete you aren't waiting for the whole method, just the part before the first await.

You can fix that by using Task.Unwrap which creates a Task<T> that represents that Task<Task<T>>>:

Task<Task> wrapperTask = Task.Factory.StartNew(...);
Task actualTask = wrapperTask.Unwrap();
Task.WaitAll(actualTask);

Answer 2

The issue with your code is that there are two tasks at play. One is the result of your Task.Factory.StartNew call, which causes the anonymous function to be executed on the thread pool. However, your anonymous function is in turn compiled to produce a nested task, representing the completion of its asynchronous operations. When you wait on your Task<Task<object>>, you're only waiting on the outer task. To wait on the inner task, you should use Task.Run instead of Task.Factory.StartNew, since it automatically unwraps your inner task:

Random rng = new Random((int)DateTime.UtcNow.Ticks);
int delay = rng.Next(1500, 15000);
Task<int> testTask = Task.Run(
    async () =>
    {
        DateTime startTime = DateTime.Now;
        Console.WriteLine("{0} - Starting test task with delay of {1}ms.", DateTime.Now.ToString("h:mm:ss.ffff"), delay);
        await Task.Delay(delay);
        Console.WriteLine("{0} - Test task finished after {1}ms.", DateTime.Now.ToString("h:mm:ss.ffff"), (DateTime.Now - startTime).TotalMilliseconds);
        return delay;
    });
Task<int>[] tasks = new[] { testTask };

Task.WaitAll(tasks);
Console.WriteLine("{0} - Finished waiting.", DateTime.Now.ToString("h:mm:ss.ffff"));

// make console stay open till user presses enter
Console.ReadLine();

Answer 3

Here, Task.WaitAll waits for the outer task and not the inner task. Use Task.Run to not have nested tasks. That's the best practices solution. Another solution is to also wait for the inner task. For example:

Task<object> testTask = Task.Factory.StartNew(
    async (obj) =>
        {
            ...
        }
    ).Unwrap();

Or:

testTask.Wait();
testTask.Result.Wait();

Answer 4

After much screwing around and hair-pulling I finally decided to get rid of the async lambda and just use the System.Threading.Thread.Sleep method, to see if that made any difference.

The new code ended up like this:

Random rng = new Random((int)DateTime.UtcNow.Ticks);
int delay = rng.Next(1500, 15000);
Task<object> testTask = Task.Factory.StartNew<object>(
    (obj) =>
        {
            DateTime startTime = DateTime.Now;
            Console.WriteLine("{0} - Starting test task with delay of {1}ms.", DateTime.Now.ToString("h:mm:ss.ffff"), (int)obj);
            System.Threading.Thread.Sleep((int)obj);
            Console.WriteLine("{0} - Test task finished after {1}ms.", DateTime.Now.ToString("h:mm:ss.ffff"), (DateTime.Now - startTime).TotalMilliseconds);
            return obj;
        },
        delay
    );
Task<object>[] tasks = new Task<object>[] { testTask };

Task.WaitAll(tasks);
Console.WriteLine("{0} - Finished waiting.", DateTime.Now.ToString("h:mm:ss.ffff"));

// make console stay open till user presses enter
Console.ReadLine();

Note: Due to removing the async keyword from the lambda method, the task type can now be simply Task<object> rather than Task<Task<object>> - you can see this change in the code above.

And, voila! It worked! I got the 'Finished waiting.' message AFTER the task completed.

Alas, I remember reading somewhere that you shouldn't use System.Threading.Thread.Sleep() in your Task code. Can't remember why; but as it was just for testing and most tasks will actually be doing something that takes time rather than pretending to be doing something that takes time, this shouldn't really be an issue.

Hopefully this helps some people out. I'm definitely not the best programmer in the world (or even close) and my code is probably not great, but if it helps someone, great! :)

Thanks for reading.

Edit: The other answers to my question explain why I had the problem I did and this answer only solved the problem by mistake. Changing to Thread.Sleep(x) had no effect. Thank you to all the people who answered and helped me out!