Chaining sequential async tasks within a ForEach loop: Is this a good way to do it?

Question

I am working on a windows service and I have two related calls that depend on each other, that I want to run asynchronously for each "pair" or "set" of calls. There are several ways to do this and I have tried a few different was and settles on this because it is convenient to have one block of code to deal with as opposed to two separate blocks with their own await Task.WhenAll() calls. In my testing this seems to work as expected but I have never chained two tasks together like this before and I am wondering if this is a good way to do it and if there might be a more appropriate way to get the same result (a single block of code).

Here's what I have. Does this look like a reasonable way to chain tasks and if not, please tell me why.

Thanks in advance. -Frank

//get all pending batches
foreach (string batchId in workload)
{
    try
    {
        // I am using this ContinueWith pattern here to aggregate the Tasks
        // which makes it more convenient to handle exceptions in one place
        Task t = bll.GetIncomingBatchAsync(batchId).ContinueWith(
            task => bll.SaveIncomingBatchAsync(task.Result),
            TaskContinuationOptions.OnlyOnRanToCompletion);

        saveBatchTasks.Add(t);
    }
    catch (Exception ex)
    {
        _logger.WriteError(ex, "ProcessWorkloadAsync error building saveBatchTasks!");
        throw ex;
    }
}

try
{
    await Task.WhenAll(saveBatchTasks);
}
catch (Exception ex)
{
    _logger.WriteError(ex, "ProcessWorkloadAsync error executing saveBatchTasks!");
    throw ex;
}

I hope that the `bll` object is thread-safe, because it is going to be accessed by multiple threads concurrently without synchronization. — Theodor Zoulias, May 20 '20 at 20:31
It should be thread safe since passed in variables are by_val and everything else is local to the class methods. The operation that uses the bll class itself runs in it's own managed thread and each call gets its own instance of bll which is wrapped in a using statement. Thank you for making me think about it though! :) — Frank Silano, May 21 '20 at 16:07

score 3 · Accepted Answer · answered May 20 '20 at 20:09

No, you should not use ContinueWith. Use await instead.

If you're worried about separating the logic across two functions, just use a local function:

//get all pending batches
foreach (string batchId in workload)
{
  try
  {
    async Task GetAndSave()
    {
      var result = await bll.GetIncomingBatchAsync(batchId);
      await bll.SaveIncomingBatchAsync(result);
    }

    saveBatchTasks.Add(GetAndSave());
  }
  catch (Exception ex)
  {
    _logger.WriteError(ex, "ProcessWorkloadAsync error building saveBatchTasks!");
    throw ex;
  }
}

Thank you Stephen, I was hoping you would chime in. :) – Frank Silano May 21 '20 at 13:55 — Frank Silano, May 21 '20 at 13:55

score 1 · Answer 2 · answered May 20 '20 at 20:22

1

Generally combining the old-school ContinueWith method with async/await is not recommended, since the later was invented to replace the former. You could use LINQ to create your tasks in one line if you want:

Task[] saveBatchTasks = workload.Select(async batchId =>
{
    var result = await bll.GetIncomingBatchAsync(batchId);
    await bll.SaveIncomingBatchAsync(result);
}).ToArray();

await Task.WhenAll(saveBatchTasks);

answered May 20 '20 at 20:22

Theodor Zoulias

34,835
7
69
104

1

Thanks Theodor. After sleeping on it I came to the same conclusion which is exactly what I was doing before I came up with the ContinueWith concept. – Frank Silano May 21 '20 at 13:56

Chaining sequential async tasks within a ForEach loop: Is this a good way to do it?

2 Answers2