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
– Theodor Zoulias, Commented May 20, 2020 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
– Frank Silano, Commented May 21, 2020 at 16:07

Stephen Cleary · Accepted Answer · 2020-05-20 20:09:29Z

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; } }

Theodor Zoulias · Accepted Answer · 2020-05-20 20:22:59Z

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);

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.

Collectives™ on Stack Overflow

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

2 Answers 2

1 Comment

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

1 Comment

Related