9

I've done this Unit Test and I don't understand why the "await Task.Delay()" doesn't wait !

   [TestMethod]
    public async Task SimpleTest()
    {
        bool isOK = false;
        Task myTask = new Task(async () =>
        {
            Console.WriteLine("Task.BeforeDelay");
            await Task.Delay(1000);
            Console.WriteLine("Task.AfterDelay");
            isOK = true;
            Console.WriteLine("Task.Ended");
        });
        Console.WriteLine("Main.BeforeStart");
        myTask.Start();
        Console.WriteLine("Main.AfterStart");
        await myTask;
        Console.WriteLine("Main.AfterAwait");
        Assert.IsTrue(isOK, "OK");
    }

Here is the Unit Test output :

Unit Test Output

How is this possible an "await" doesn't wait, and the main thread continues ?

Elo
  • 2,234
  • 22
  • 28
  • It's slightly unclear what you are trying to achieve. Can you please add your expected output? – OlegI Nov 15 '19 at 09:07
  • 1
    The test method is very clear - isOK is expected to be true – Sir Rufo Nov 15 '19 at 09:09
  • There's no reason to create a non-started task. Tasks aren't threads, they *use* threads. What are you trying to do? Why not use `Task.Run()` after the first `Console.WriteLine`? – Panagiotis Kanavos Nov 15 '19 at 09:20
  • If you think that `Start` gives you control over the task's lifetime, it doesn't. Whether you use `Task.Run` or `Start`, the task will have to wait until a threadpool thread is availavble to execute it. – Panagiotis Kanavos Nov 15 '19 at 09:22
  • @PanagiotisKanavos In another project, I need to create a big queue of tasks, and then to execute them one by one. The task client could await its task, but he doesn't know when it will be executed. So I need to create not executed tasks. I want to say I don't want to Enqueue all tasks to ThreadPool, because they would drown other more important Tasks. – Elo Nov 15 '19 at 09:24
  • 1
    @Elo you just described threadpools. You *don't* need a pool of tasks to implement a job queue. You need a queue of jobs, eg Action objects – Panagiotis Kanavos Nov 15 '19 at 09:27
  • @Elo `I don't want to Enqueue all tasks to ThreadPool` but that's *exactly* what you do. `Start` enqueues a task to run on a threadpool thread. – Panagiotis Kanavos Nov 15 '19 at 09:28
  • 2
    @Elo what you try to do is already available in .NET eg through TPL Dataflow classes like ActionBlock, or the newer System.Threading.Channels classes. You can create an ActionBlock to receive and process messages using one or more concurrent tasks. All blocks have input buffers with configurable capacity. The DOP and capacity allow you to control concurrency, throttle requests and implement backpressure - if too many messages are queued, the producer awaits – Panagiotis Kanavos Nov 15 '19 at 09:30
  • @PanagiotisKanavos In this sample, yes, I start the task and enqueue it, but in my project let's say I have a List and I take tasks one by one to Start them. – Elo Nov 15 '19 at 09:32
  • @PanagiotisKanavos I can't just queue some Action because caller couldn't be notified when it's finished. If the caller creates the Task, it could await it even if I execute it later. – Elo Nov 15 '19 at 09:48
  • In that case you can give the caller a Taskcompletionsource-generated task that gets completed from inside the action job. Or provide a callback, that may well use its own TCS. Or include the continuation with the job instead of waiting for a response. With channels or blocks, the next processing step is handled by another block or worker, not the caller. – Panagiotis Kanavos Nov 15 '19 at 09:53

3 Answers3

11

new Task(async () =>

A task does not take a Func<Task>, but an Action. It will call your asynchronous method and expect it to end when it returns. But it does not. It returns a task. That task is not awaited by the new task. For the new task, the job is done once the method returned.

You need to use the task that already exists instead of wrapping it in a new task:

[TestMethod]
public async Task SimpleTest()
{
    bool isOK = false;

    Func<Task> asyncMethod = async () =>
    {
        Console.WriteLine("Task.BeforeDelay");
        await Task.Delay(1000);
        Console.WriteLine("Task.AfterDelay");
        isOK = true;
        Console.WriteLine("Task.Ended");
    };

    Console.WriteLine("Main.BeforeStart");
    Task myTask = asyncMethod();

    Console.WriteLine("Main.AfterStart");

    await myTask;
    Console.WriteLine("Main.AfterAwait");
    Assert.IsTrue(isOK, "OK");
}
nvoigt
  • 75,013
  • 26
  • 93
  • 142
5

The problem is that you are using the non-generic Task class, that is not meant to produce a result. So when you create the Task instance passing an async delegate:

Task myTask = new Task(async () =>

...the delegate is treated as async void. An async void is not a Task, it cannot be awaited, its exception cannot be handled, and it's a source of thousands of questions made by frustrated programmers here in StackOverflow and elsewhere. The solution is to use the generic Task<TResult> class, because you want to return a result, and the result is another Task. So you have to create a Task<Task>:

Task<Task> myTask = new Task<Task>(async () =>

Now when you Start the outer Task<Task> it will be completed almost instantly because its job is just to create the inner Task. You'll then have to await the inner Task as well. This is how it can be done:

myTask.Start(TaskScheduler.Default);
Task myInnerTask = await myTask;
await myInnerTask;

You have two alternatives. If you don't need an explicit reference to the inner Task then you can just await the outer Task<Task> twice:

await await myTask;

...or you can use the built-in extension method Unwrap that combines the outer and the inner tasks into one:

await myTask.Unwrap();

This unwrapping happens automatically when you use the much more popular Task.Run method that creates hot tasks, so the Unwrap is not used very often nowadays.

In case you decide that your async delegate must return a result, for example a string, then you should declare the myTask variable to be of type Task<Task<string>>.

Note: I don't endorse the use of Task constructors for creating cold tasks. As a practice is generally frowned upon, for reasons I don't really know, but probably because it is used so rarely that it has the potential of catching other unaware users/maintainers/reviewers of the code by surprise.

General advice: Be careful everytime you are supplying an async delegate as an argument to a method. This method should ideally expect a Func<Task> argument (meaning that understands async delegates), or at least a Func<T> argument (meaning that at least the generated Task will not be ignored). In the unfortunate case that this method accepts an Action, your delegate is going to be treated as async void. This is rarely what you want, if ever.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • 1
    Note: You cannot start unwrapped Task. If you call Start() after Unwrap(), it will throw InvalidOperationException with message "start may not be called on a promise-style task". – Sarrus Aug 27 '20 at 11:09
1
 [Fact]
        public async Task SimpleTest()
        {
            bool isOK = false;
            Task myTask = new Task(() =>
            {
                Console.WriteLine("Task.BeforeDelay");
                Task.Delay(3000).Wait();
                Console.WriteLine("Task.AfterDelay");
                isOK = true;
                Console.WriteLine("Task.Ended");
            });
            Console.WriteLine("Main.BeforeStart");
            myTask.Start();
            Console.WriteLine("Main.AfterStart");
            await myTask;
            Console.WriteLine("Main.AfterAwait");
            Assert.True(isOK, "OK");
        }

enter image description here

BASKA
  • 311
  • 4
  • 15
  • 3
    You realize that without the `await`, the task delay will not actually delay that task, right? You removed functionality. – nvoigt Nov 15 '19 at 09:28
  • I just tested, @nvoigt is right: Elapsed time: 0:00:00,0106554. And we see in your screenshot : "elapsed time : 18 ms", it should be >= 1000 ms – Elo Nov 15 '19 at 09:42
  • Yes you are right, i update my response. Tnx. After you comments i solve this with minimal changes. :) – BASKA Nov 15 '19 at 10:16
  • 1
    Good idea to use wait() and not await from inside a Task ! thanks ! – Elo Nov 15 '19 at 11:53