none
Novice question why this is happening: Tasks.WaitAll vs. Parallel.ForEach

    Question

  • [I am working in C# with Visual Studio 2008 and the June 08 CTP of the Parallel Extensions to the .NET Framework.]

    To start learning the extensions, I took some code that I had laying around that solved Project Euler problems concurrently using background worker threads and tried to convert it to use the framework. Right off the bat I have run into a snag.

    When I execute the following, all is right with the world (and with a slightly faster execution time than my original background worker thread based code)

    Parallel.ForEach(problems, o => Problem.InvokeProblem(o));
    Where problems is a list of the problems to be solved (an object with an int ID and a delegate to the static method to be called) and the static method InvokeProblem on Problem solves a given problem and dumps information to the console (the solution, elapsed time, and ID).

    My problem is when I try this:

    List<Task> tasks = new List<Task>();
    foreach (ProblemData pd in problems)
    {
      tasks.Add(Task.Create((o) => Problem.InvokeProblem(pd)));
    }
    Task.WaitAll(tasks.ToArray());
    What happens is the last problem in the list is the only one solved; the number of times equal to the list count. Obviously I am not creating the task correctly, but I am not seeing what I have done wrong. Surely it is something trivial that I am doing wrong.

    As an aside, the reason I looked at the second method is my curiosity as to how to quite once one of the tasks completed (not caring which one and canceling all the others).

    Thanks
    Saturday, May 23, 2009 5:52 PM

Answers

  • Hi Randy-

    Looks like you're running up against how C# handles closures... try the following instead:

        List<Task> tasks = new List<Task>();
        foreach (ProblemData pd in problems)
        {
          ProblemData temp = pd;
          tasks.Add(Task.Create((o) => Problem.InvokeProblem(temp)));
        }
        Task.WaitAll(tasks.ToArray());

    The issue is that the C# compiler treats the "pd" iteration variable as being outside of the loop, and due to capture semantics, every iteration of the loop then shares the same variable.  Thus, all the tasks refer to the same variable, which means that if the loop has quickly iterated to the last value, but the time the tasks run, they'll all be pointing to the last value.  By changing the loop to make a copy into a variable inside the loop, each Task gets it own copy.  Alternatively, you could just pass the value into the loop:

        List<Task> tasks = new List<Task>();
        foreach (ProblemData pd in problems)
        {
          tasks.Add(Task.Create(o => Problem.InvokeProblem((ProblemData)o), pd));
        }
        Task.WaitAll(tasks.ToArray());

    Hope that helps.

    Saturday, May 23, 2009 6:14 PM

All replies

  • Hi Randy-

    Looks like you're running up against how C# handles closures... try the following instead:

        List<Task> tasks = new List<Task>();
        foreach (ProblemData pd in problems)
        {
          ProblemData temp = pd;
          tasks.Add(Task.Create((o) => Problem.InvokeProblem(temp)));
        }
        Task.WaitAll(tasks.ToArray());

    The issue is that the C# compiler treats the "pd" iteration variable as being outside of the loop, and due to capture semantics, every iteration of the loop then shares the same variable.  Thus, all the tasks refer to the same variable, which means that if the loop has quickly iterated to the last value, but the time the tasks run, they'll all be pointing to the last value.  By changing the loop to make a copy into a variable inside the loop, each Task gets it own copy.  Alternatively, you could just pass the value into the loop:

        List<Task> tasks = new List<Task>();
        foreach (ProblemData pd in problems)
        {
          tasks.Add(Task.Create(o => Problem.InvokeProblem((ProblemData)o), pd));
        }
        Task.WaitAll(tasks.ToArray());

    Hope that helps.

    Saturday, May 23, 2009 6:14 PM
  • Stephen,

    That was it. Thank you!
    Saturday, May 23, 2009 6:21 PM