Converting loop to tasks

Question

I have the following synchronous code:

foreach ( var step in result ) {
  step.Run();
}

I tried to convert it to tasks but I failed to do so. I tried to convert it using Task.WhenAll like this (and I did append async to the method signature):

var tasks = new List<Task>();
foreach ( var step in result ) {
    tasks.Add( new Task( () => step.Run() ) );
}
await Task.WhenAll( tasks );

This returns immediately and doesn't execute the Run() method. Then I tried to convert it to the following code:

var tasks = new List<Task>();
foreach ( var step in result ) {
    tasks.Add( new Task( () => step.Run() ) );
}
var task = Task.WhenAll( tasks );
task.Wait();

This blocks forever. However, when I create a within the loop it works:

foreach ( var step in result ) {
    var t = Task.Run( () => step.Run() );
    t.Wait();
}

If I use instead await Task.Run( () => step.Run() ); it awaits only the first one and resumes the main thread.

The run method looks like this:

public async void Run() {
    var result = Work();
    if ( null != result && result.Count > 0 ) {
        var tasks = new List<Task>();
        foreach ( var step in result ) {
            await Task.Run( () => step.Run() );
        }
    }
}

All steps implement a Work() method (which is abstract in a base class). My first step looks like this:

class NoWorkStep : WorkerStep {
    protected override IList<WorkerStep> Work() {
        Console.WriteLine( "HERE" );
        List<WorkerStep> newList = new List<WorkerStep>();
        for ( int i = 0; i < 10; i++ ) {
            newList.Add( new NoWorkStep2() );
        }
        return newList;
    }
}

And my second step looks like this:

class NoWorkStep2 : WorkerStep {
    protected override IList<WorkerStep> Work() {
        Console.WriteLine( "HERE-2" );
        return new List<WorkerStep>();
    }
}

I simple create an instance of NoWorkStep and call instance.Run().

Where do I have a problem with executing the steps with Task.WhenAll?

Edit: Calling code after I changed the Run method to async Task RunAsync:

private static async void doIt() {
  var step = new NoWorkStep();
  await step.RunAsync();
}

Don't use `new Task`. The reason your fourth example works is not because the `Wait` is inside the cycle - it works because you're using `Task.Run` instead of `new Task`. — Luaan, Apr 21 '15 at 13:19

Yuval Itzchakov · Accepted Answer · 2015-04-21T07:38:32.747

20

Lets map out the problems with your code:

new Task(() => step.Run())

This returns a cold Task, meaning the Task isn't actually started. In order for it to start you would need to call:

new Task(() => step.Run()).Start)

But, you shouldn't use new Task anyway, you should use Task.Run.

If I use instead await Task.Run( () => step.Run() ); it awaits only the first one and resumes the main thread.

That is because Run is async void which cannot be awaited. async void is ment to be used only in top level event handlers, where this clearly isn't the case here.

If you want to await on until all the tasks are completed, you can do that following:

public async Task RunAsync() 
{
    var result = Work();
    var stepTasks = result.Select(step => Task.Run(() => step.Run()));
    await Task.WhenAll(steps);
}

This will guarantee all tasks have completed execution once RunAsync finishes.

edited Apr 21 '15 at 07:38

answered Apr 21 '15 at 06:44

Yuval Itzchakov

146,575
32
257
321

I changed `async void Run` to `async Task RunAsync()` and used your code snippet. I call RunAsync using `await step.RunAsync();` and unfortunately sometimes it prints out ten times HERE-2, sometimes two times sometimes no HERE-2. – Sascha Apr 21 '15 at 07:34
@Sascha Who calls `doIt`? and how? – Yuval Itzchakov Apr 21 '15 at 07:41
I call `doIt` from the `static void Main` – Sascha Apr 21 '15 at 07:41
Well, `Main` will terminate before `doIt` has a chance to complete. You need to make `doIt` return a `Task` as well and use `doIt().Wait()` if you're running inside a console application. – Yuval Itzchakov Apr 21 '15 at 07:42
Thanks. Works now. You're right and the moment I read your comment it was clear. Thanks for your help – Sascha Apr 21 '15 at 07:44

RagtimeWilly · Answer 2 · 2015-04-21T06:58:03.460

6

You don't seem to be starting the tasks.

Try:

var tasks = new List<Task>();

foreach (var step in result) 
{
    var t = new Task(() => step.Run());
    t.Start();
    tasks.Add(t);
}

Task.WhenAll(tasks);

edited Apr 21 '15 at 06:58

answered Apr 21 '15 at 06:45

RagtimeWilly

5,265
3
25
41

2

Just call Task.Run, there's no need to create a cold task just to start it in the next line – Panagiotis Kanavos Apr 21 '15 at 06:56
I was just trying to illustrate why the code in the question wasn't working. – RagtimeWilly Apr 21 '15 at 07:00
Cold tasks was the real problem with the original code. It's easy to forget the call to Start and they offer nothing over 'Task.Run` or 'Task.Factory.StartNew'. – Panagiotis Kanavos Apr 21 '15 at 07:02

score 6 · Answer 3 · answered Apr 21 '15 at 07:06

6

You can use Parallel.ForEach.

Parallel.ForEach(result, step => step.Run());

This way you don't even muck around with the lower level parts of the Parallel Framework.

answered Apr 21 '15 at 07:06

jdphenix

15,022
3
41
74

2

This would work and maybe I end with this, but the aync-await pattern should work too, and I like to understand where I have my mistake. – Sascha Apr 21 '15 at 07:39
Or indeed `result.AsParallel().ForAll(step => step.Run());`, but you wouldn't really use PLINQ unless you had a actual query. – Nathan Cooper Apr 21 '15 at 14:32
@NathanCooper You're right, it depends on the type of `result` if it makes sense or not. Both work, but I like `ForEach()` here because it preserves the intent of the initial code snippet. – jdphenix Apr 21 '15 at 17:54

Converting loop to tasks

3 Answers3