3

Say I have the following action method:

[HttpPost]
public async Task<IActionResult> PostCall()
{
    var tasks = new List<Task<bool>>();
    for (int i = 0; i < 10; i++)
        tasks.Add(Manager.SomeMethodAsync(i));

    // Is this line necessary to ensure that all tasks will finish successfully?
    Task.WaitAll(tasks.ToArray());

    if (tasks.Exists(x => x.Result))
        return new ObjectResult("At least one task returned true");
    else
        return new ObjectResult("No tasks returned true");
}

Is Task.WaitAll(tasks.ToArray()) necessary to ensure that all tasks will finish successfully? Will the tasks whose Result happened not to get accessed by the Exists finish their execution in the background successfully? Or is there a chance that some of the tasks (that weren't waited for) get dropped since they would not be attached to the request? Is there a better implementation I'm missing?

yazanpro
  • 4,512
  • 6
  • 44
  • 66
  • 2
    See `HostingEnvironment.QueueBackgroundWorkItem` in https://stackoverflow.com/questions/17577016/long-running-task-in-webapi – opewix Jan 10 '19 at 22:05

2 Answers2

3

Yes, the tasks are not guaranteed to complete unless something waits for them (with something like an await)

In your case, the main change you should make is making the Task.WaitAll

await Task.WhenAll(tasks);

So it is actually asynchronous. If you just want to wait for a task to return, use WhenAny instead.

yazanpro
  • 4,512
  • 6
  • 44
  • 66
BradleyDotNET
  • 60,462
  • 10
  • 96
  • 117
  • Thank you. Just to clarify, there is `await` in the implementation of `Manager.SomeMethodAsync`. Does that make a difference? – yazanpro Jan 10 '19 at 22:08
  • 4
    Using await doesn't *start* the task. The task starts when you call `SomeMethodAsync` etc. There's just no guarantee it will have *completed*. – Jon Skeet Jan 10 '19 at 22:10
  • @JonSkeet I swear I've had some not even start, but I could have interpreted those wrong. Updated the answer. – BradleyDotNET Jan 10 '19 at 22:13
  • @BradleyDotNET: If you see that happen again, it'd be great to see a new question about it - then feel free to drop me an email to bring it to my attention. But fundamentally `await` is about consuming a task (or other asynchronous operation) rather than changing the behavior of the task itself. – Jon Skeet Jan 11 '19 at 13:54
  • And awaiting or not a task has no effect in it completing or not. – Paulo Morgado Jan 11 '19 at 15:46
  • @PauloMorgado Are you saying that `await Task.WhenAll(tasks);` will not ensure the completion of all tasks? How is that? – yazanpro Jan 11 '19 at 15:48
  • Kind of. the next line of code will only be executed after all task have been completed. But not awaiting them does not mean that they don't complete. – Paulo Morgado Jan 11 '19 at 15:52
  • 1
    @yazanpro: It's technically possible for a method to return an unstarted task (e.g. a `Task` created through its constructor), but that goes against today's conventions: "If a TAP method internally uses a task’s constructor to instantiate the task to be returned, the TAP method must call Start on the Task object before returning it. Consumers of a TAP method may safely assume that the returned task is active and should not try to call Start on any Task that is returned from a TAP method." – Douglas Jan 11 '19 at 18:38
3

Under your provided implementation, the Task.WaitAll call blocks the calling thread until all tasks have completed. It would only proceed to the next line and perform the Exists check after this has happened. If you remove the Task.WaitAll, then the Exists check would cause the calling thread to block on each task in order; i.e. it first blocks on tasks[0]; if this returns false, then it would block on tasks[1], then tasks[2], and so on. This is not desirable since it doesn't allow for your method to finish early if the tasks complete out of order.

If you only need to wait until whichever task returns true first, then you could use Task.WhenAny. This will make your asynchronous method resume as soon as any task completes. You can then check whether it evaluated to true and return success immediately; otherwise, you keep repeating the process for the remaining collection of tasks until there are none left.

If your code was running as an application (WPF, WinForms, Console), then the remaining tasks would continue running on the thread pool until completion, unless the application is shut down. Thread-pool threads are background threads, so they won't keep the process alive if all foreground threads have terminated (e.g. because all windows were closed).

Since you're running a web app, you incur the risk of having your app pool recycled before the tasks have completed. The unawaited tasks are fire-and-forget and therefore untracked by the runtime. To prevent this from happening, you can register them with the runtime through the HostingEnvironment.QueueBackgroundWorkItem method, as suggested in the comments.

[HttpPost]
public async Task<IActionResult> PostCall()
{
    var tasks = Enumerable
        .Range(0, 10)
        .Select(Manager.SomeMethodAsync)
        .ToList();

    foreach (var task in tasks)
        HostingEnvironment.QueueBackgroundWorkItem(_ => task);

    while (tasks.Any())
    {
        var readyTask = await Task.WhenAny(tasks);
        tasks.Remove(readyTask);
        if (await readyTask)
            return new ObjectResult("At least one task returned true");
    }

    return new ObjectResult("No tasks returned true");
}
Douglas
  • 53,759
  • 13
  • 140
  • 188