10

Possible Duplicate:
C# Captured Variable In Loop

I am pretty new to multi-threading programming. When I ran the code below and only the last child got executed. Can some one tell me what happened? Thank you very much.

private void Process()
{
    Dictionary<int, int> dataDict = new Dictionary<int, int>();
    dataDict.Add(1, 2000);
    dataDict.Add(2, 1000);
    dataDict.Add(3, 4000);
    dataDict.Add(4, 3000);

    foreach (KeyValuePair<int, int> kvp in dataDict)
    {
        Console.WriteLine("Ready for [" + kvp.Key.ToString() + "]");
        Task.Factory.StartNew(() => DoSomething(kvp.Value, kvp.Key));
    }

private static void DoSomething(int waitTime, int childID)
{
    {               
        Console.WriteLine("Start task [" + childID.ToString() + "]");
        Thread.Sleep(waitTime);
        Console.WriteLine("End task [" + childID.ToString() + "]");
    }
}

Output


Ready for [1]
Ready for [2]
Ready for [3]
Ready for [4]
Start task [4]
Start task [4]
Start task [4]
Start task [4]
End task [4]
End task [4]
End task [4]
End task [4]
Community
  • 1
  • 1
Dreteh
  • 387
  • 2
  • 4
  • 11

2 Answers2

12

By using the loop variable in your lambda, all of the effectively refer to the same variable, which is the last item of your dictionary at the time they run.

You need to assign the loop variable to another variable local to the loop before passing it to a lambda. Do this:

foreach (KeyValuePair<int, int> kvp in dataDict)
{
    var pair = kvp;
    Console.WriteLine("Ready for [" + pair.Key.ToString() + "]");
    Task.Factory.StartNew(() => DoSomething(pair.Value, pair.Key));
}

EDIT: It seems this little pitfall is fixed in C#5. That's why it might work for others ;) See comment by labroo

Botz3000
  • 39,020
  • 8
  • 103
  • 127
  • 2
    people should really explain why they downvoted, seems like a reasonable explanation to me – Nadir Muzaffar Mar 08 '12 at 07:18
  • 2
    Why the downvote, I dont know if this is the solution, but it is a valid change....... http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx – labroo Mar 08 '12 at 07:20
  • Was wondering, too. Can't see anything wrong with my answer. – Botz3000 Mar 08 '12 at 07:20
  • btw, what Botz3000's answer is implying is that all of the threads run...however, by the time they get the chance to run, kvp == 4 – Nadir Muzaffar Mar 08 '12 at 07:23
  • I figured that too, but I just don't understand why. – Dreteh Mar 08 '12 at 07:24
  • for that, you'll need to read up a bit about how lambda functions capture scope – Nadir Muzaffar Mar 08 '12 at 07:27
  • **Dreteh**, read the link posted by **labroo** to understand. By using the loop variable in your lambda, all of them effectively refer to the same variable, which is the last item of your dictionary at the time they run. – Botz3000 Mar 08 '12 at 07:38
  • You should explain the problem and solution if you are going to duplicate an answer from an already established question – meandmycode Mar 08 '12 at 07:41
  • Thank you guys. Lobroo's link explain all. – Dreteh Mar 08 '12 at 08:05
1

You can prevent that behavior by assigning kvp to a local variable in the for loop and pass the variables fields Key and Value to the DoSomething method.

saintedlama
  • 6,838
  • 1
  • 28
  • 46