1

I am trying to spawn threads in a for each loop using a copy of the value in a dict.

My initial understanding was that the foreach would create a new scope, and led to:

Dictionary<string, string> Dict = new Dictionary<string, string>() { { "sr1", "1" }, { "sr2", "2" } };
foreach (KeyValuePair<string, string> record in Dict) {
    new System.Threading.Timer(_ =>
    {
        Console.WriteLine(record.Value);
    }, null, TimeSpan.Zero, new TimeSpan(0, 0, 5));
}

which writes

1
2
2
2

instead of (expected):

1
2
1
2

So I tried cloning the kvp in the foreach:

KeyValuePair<string, string> tmp = new KeyValuePair<string, string>(record.Key, record.Value);

but that renders the same result.

I've also tried it with System.Parallel.ForEach but that seems need values that are not dynamic, which is a bit of a train smash for my dictionary.

How can I iterate through my Dictionary with threads?

quetzalcoatl
  • 32,194
  • 8
  • 68
  • 107
Alex
  • 5,674
  • 7
  • 42
  • 65
  • 1
    I can't see how the code you've given would work at all, given that you don't start the timer... and you've got 2 entries in your dictionary, not the 4 you're printing out. If you can create a short but *complete* program demonstrating the problem, that would really help. – Jon Skeet Jul 15 '13 at 15:41
  • 1
    First, why would it write 1/2/1/2 when all dictio entries have Value of 1? Then, why'd you try to put two dictio entries with the same "sr" key? It would immediatelly throw.. – quetzalcoatl Jul 15 '13 at 15:41
  • @quetzalcoatl Whoops, the 2nd sr was meant to be "2" to demonstrate. – Alex Jul 15 '13 at 15:43
  • 1
    possible duplicate of [Is there a reason for C#'s reuse of the variable in a foreach?](http://stackoverflow.com/questions/8898925/is-there-a-reason-for-cs-reuse-of-the-variable-in-a-foreach) -- even if not exactly the duplicate, the **question** under that link perfectly describes the problem and its cause and effects. Please check it out! – quetzalcoatl Jul 15 '13 at 15:54

1 Answers1

7

The problem is closure over your lambda, the way to fix is is to add a local variable inside the for loop

Dictionary<string, string> Dict = new Dictionary<string, string>() { { "sr1", "1" }, { "sr2", "2" } };
foreach (KeyValuePair<string, string> record in Dict) {

    var localRecord = record;
    new System.Threading.Timer(_ =>
    {
        Console.WriteLine(localRecord.Value);
    }, null, TimeSpan.Zero, new TimeSpan(0, 0, 5));
}

What is happening in your version is it captures the variable record not the value of the variable record. So when the timer runs the 2nd time it uses the "current value" of record which is the 2nd element in the array.

Behind the scenes this is what is kinda what is happening in your version of the code.

public void MainFunc()
{
    Dictionary<string, string> Dict = new Dictionary<string, string>() { { "sr1", "1" }, { "sr2", "2" } };
    foreach (KeyValuePair<string, string> record in Dict) {

        _recordStored = record;
        new System.Threading.Timer(AnnonFunc, null, TimeSpan.Zero, new TimeSpan(0, 0, 5));
    }
}

private KeyValuePair<string, string> _recordStored;

private void AnnonFunc()
{
    Console.WriteLine(_recordStored.Value);
}

See how when your function ran it for the first itteration had the correct version of _recordStored, but after _recordStored got overwritten it will only show the last set value. By creating a local variable it does not do that overwriting.

A way to imagine it (I am not sure how I could represent it in a code example) is it creates _recordStored1 the first loop, _recordStored2 the 2nd loop, and so on. The function uses the correct version of _recordStored# for when it calls the the function.

Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • 1
    FYI: [Is there a reason for reuse of the variable in a foreach](http://stackoverflow.com/questions/8898925/is-there-a-reason-for-cs-reuse-of-the-variable-in-a-foreach) describes all problems with foreach's autovariable quite well and in-depth, and even comes with EricLippert's comments ;) – quetzalcoatl Jul 15 '13 at 15:55
  • Pointer + Concurrency bugs are fun! Why the hell do I need to specify a type for the foreach if its just a pointer anyway though? – Alex Jul 15 '13 at 16:04
  • Alternatively... one could just use C#5 were IIRC they changed the behaviour of foreach :) "In C# 5, the loop variable of a foreach will be logically inside the loop, and therefore closures will close over a fresh copy of the variable each time." http://blogs.msdn.com/b/ericlippert/archive/2009/11/16/closing-over-the-loop-variable-part-two.aspx – NPSF3000 Jul 15 '13 at 16:20
  • @Alex The reason you have to specify the type is because Dictionary only implments `IEnumerable` not `IEnumberable`, why it does not do it, I do not know. Also it is not a pointer, pointers are a very different thing and use the `IntPtr` datatype. – Scott Chamberlain Jul 15 '13 at 16:25