Correct approach to wait for multiple async methods to complete

  • I have an IWorkflow interface defined as follows:



    public interface IWorkflow
    {
    Task ConfigureAsync();
    Task StartAsync();
    Task StopAsync();
    }


    And I have an Engine class:



    public sealed class Engine : IEngine
    {
    private readonly List<IWorkflow> workflows = new List<IWorkflow>();

    public Engine(IEnumerable<IWorkflow> workflows)
    {
    this.workflows.AddRange(workflows);
    }

    public void Start()
    {
    var configureTasks = this.workflows.Select(w => w.ConfigureAsync()).ToArray();
    Task.WaitAll(configureTasks);

    var startTasks = this.workflows.Select(w => w.StartAsync()).ToArray();
    Task.WaitAll(startTasks);
    }

    public void Stop()
    {
    var stopTasks = this.workflows.Select(w => w.StopAsync()).ToArray();
    Task.WaitAll(stopTasks);
    }
    }


    Is this the correct way for the Engine to invoke in parallel the configure method on all workflows and then once all are completed, invoke in parallel the start method on all workflows?


    Is there any reason why `Engine` isn't also asynchronous?

    I don't see a need for it to be as there is only one `Engine` and it starts on the app thread. I'm updating an old .NET 2.0 app so I'm just getting to grips with Tasks and async/await & wanted to confirm I was using it in the correct way.

    If you mean it's a GUI app, then I think not locking up the GUI while the engine starts or stops is a good reason for that.

    It's a console app running in TopShelf

  • almaz

    almaz Correct answer

    8 years ago

    Your code is absolutely correct in case when you want to start workflows only when all of them are configured.



    But if you want to start each workflow once it's configured (independently from other workflows) then it might be a good idea to use continuations... In .NET 4.5 it would look like this:



    public sealed class Engine : IEngine
    {
    private readonly List<IWorkflow> _workflows;

    public Engine(IEnumerable<IWorkflow> workflows)
    {
    _workflows = new List<IWorkflow>(workflows);
    }

    private async Task RunWorkflow(IWorkflow workflow)
    {
    await workflow.ConfigureAsync();
    await workflow.StartAsync();
    }

    public void Start()
    {
    var startTasks = this._workflows.Select(RunWorkflow).ToArray();
    Task.WaitAll(startTasks);
    }

    public void Stop()
    {
    var stopTasks = _workflows.Select(w => w.StopAsync()).ToArray();
    Task.WaitAll(stopTasks);
    }
    }


    Also I would suggest to use CancellationToken to stop asynchronous processing.



    UPDATE Based on comments it is really needed to wait for all workflows to be configured before starting them. So cancellable implementation can look like this:



    public interface IWorkflow
    {
    Task ConfigureAsync(CancellationToken token);
    Task StartAsync(CancellationToken token);
    }

    public sealed class Engine : IEngine
    {
    private readonly List<IWorkflow> _workflows;
    private readonly CancellationTokenSource _cancellationTokenSource;
    private Task _mainTask;

    public Engine(IEnumerable<IWorkflow> workflows)
    {
    _workflows = new List<IWorkflow>(workflows);
    _cancellationTokenSource = new CancellationTokenSource();
    }

    private async Task RunWorkflows()
    {
    await Task.WhenAll(_workflows.Select(w => w.ConfigureAsync(_cancellationTokenSource.Token)));
    if (_cancellationTokenSource.IsCancellationRequested)
    return;
    await Task.WhenAll(_workflows.Select(w => w.StartAsync(_cancellationTokenSource.Token)));
    }

    public void Start()
    {
    _mainTask = RunWorkflows();
    _mainTask.Wait();
    }

    public void Stop()
    {
    _cancellationTokenSource.Cancel();
    var mainTask = _mainTask;
    if (mainTask != null)
    mainTask.Wait();
    }
    }

    The idea as it stands is to start quickly but only if all workflows configure successfully.

    Then your solution is correct. Take into account that Engine.Start method will finish only all workflows are configured and started.

    I think your suggestion to use `CancellationToken` is not right. Cancellation is something exceptional (and `await`ing a canceled `Task` throws an exception). Stopping some long-running process is not exceptional.

    @svick CancellationToken is a recommended way to stop asynchronous processing, it **does not** throw exceptions and is not something exceptional. You probably confuse it with `Thread.Abort`, `CancellationToken.ThrowIfCancellationRequested` or `Task.Wait(CancellationToken)` which do throw exceptions. See the link I've included in my post for example of usage.

    But `ThrowIfCancellationRequested()` is the recommended way of using `CancellationToken`, AFAIK. Also, `Task` has a special status `Canceled` and when you `Wait()` or `await` such `Task`, it will throw an exception.

    @svick If you _can_ throw exception doesn't mean you _should_ :). `OperationCanceledException` should be thrown if you want to notify the `await`'er that operation didn't complete, e.g. if order was not posted. But if logically it's ok to stop processing (e.g. you're calculating PI and once you detect that cancellation was requested you just return current approximation of PI with precision) then you don't need to throw exception, you just finish the task, and status would be `RanToCompletion`. See Task Cancellation for more details

    I don't think the cancellation would actually work in your example as the `Start` method is blocking so you can't actually stop it and cancel until it's all started which unless I'm missing something means there isn't a task to actually cancel.

    Implementation corresponds to your logic (`Engine.Start` method is also blocking in your code). `_mainTask` will get a Task object assigned as soon as all workflows return their tasks via `IWorkflow.ConfigureAsync`, that is very early. Once the `Stop` method is called we notify everyone that operation should be cancelled, and also waits on `_mainTask` as it will signal when all workflows noticed the cancellation. One thing we can actually add to protect us from incorrectly written workflows is not to call `StartAsync` if cancellation is already requested

    What I mean, is because your `Start` method doesn't return until `_mainTask` has completed at which point all configuration and start tasks will also be completed. So it's not possible to call the Stop method and therefore request cancellation while any tasks are actually still running.

    How do you call `Stop` in your sample where `Start` method also blocks until all configuration and start tasks completed?

    I don't the `Engine` runs as a windows service which is hosted in `TopShelf`.

    So the same would be with my example: `Start` blocks until all workflows are configured and started, if someone calls `Stop` all workflows that are asynchronously running in `ConfigureAsync` or `StartAsync` will be notified (they should periodically observe the `CancellationToken` status or use other options like `CancellationToken.Register` to be notified) and thus should stop their work. Please read the manual on cancelling tasks

    You're missing the point, the tasks which are used in `Configure` and `Start` are run to completion however long that takes because the `Start` method doesn't return until the tasks have all completed. This means that they cannot be cancelled by the `Stop` method as the `Stop` method cannot be called until the tasks have all completed.

    I'm afraid you're missing the point :). the `_mainTask` represents all the required work of configuring, starting tasks and waiting for their completion. It is initialized immediately without any waiting. `CancellationTokenSource` is also available immediately. Whoever calls `Stop` will be able to notify everyone to stop the processing. If your code runs in a single thread and thus can't call `Stop` because it's waiting for `Start` to finish - the same issue stands in original code. It can be easily avoided but you didn't specify this as an issue in your question

License under CC-BY-SA with attribution


Content dated before 7/24/2021 11:53 AM