C# simple multithreading - why is it not producing the correct results?

Question

can someone tell me why the below is not producing the correct results? It is giving me 1233 when I expected 0123.

    public static readonly object locker = new object();
    public static List<int> queue = new List<int>();

    public static void calculate(int input)
    {
        Thread.Sleep(1000);
        lock (locker)
        {
            queue.Add(input);
        }
    }

    [TestMethod]
    public void TestT()
    {
        int[] _intList = new int[] { 0, 1, 2, 3 };
        List<Thread> _threadList = new List<Thread>();
        foreach (int num in _intList)
        {
            Thread t = new Thread(() => calculate(num));
            t.Start();
            _threadList.Add(t);
        }

        foreach (Thread t in _threadList) { t.Join(); }

        foreach (var t in queue)
        {
            Console.WriteLine(t);
        }
    }

When I change it to use a copy of the _intList variable instead, I get the correct results of 0123. Can someone tell me why this is happening? Is it being cached somewhere?

        foreach (int num in _intList)
        {
            int testNum = num;
            Thread t = new Thread(() => calculate(testNum));
            t.Start();
            _threadList.Add(t);
        }

[Closing over the loop variable considered harmful](http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx) or [C# - The foreach identifier and closures](http://stackoverflow.com/questions/512166/c-sharp-the-foreach-identifier-and-closures) — crashmstr, Sep 08 '14 at 19:28
Why would you expect the results to be 0123? The threads can be scheduled in any order... — Steve Ruble, Sep 08 '14 at 19:29
@SteveRuble You would expect those numbers in any order, what you get is some numbers missing and some numbers repeated, due to improper usage of a closure. — Servy, Sep 08 '14 at 19:31
@SteveRuble If that were the issue you wouldn't be getting the same value multiple times. In his second version the order isn't guaranteed, but at least the same number doesn't get processed twice. — Ronan Thibaudau, Sep 08 '14 at 19:31
@DanielA.White What would the C# version have to do with anything in this case? — Ronan Thibaudau, Sep 08 '14 at 19:31
@RonanThibaudau they changed the way that foreach loops are compile in the 5.0 compiler — Daniel A. White, Sep 08 '14 at 19:32
@DanielA.White Good to know, does this mean in C# 5.0 & higher both of his examples are equal? — Ronan Thibaudau, Sep 08 '14 at 19:33

score 3 · Answer 1 · answered Sep 08 '14 at 19:30

When you're passing a variable to a lambda expression it gets captured by the expression. So it's not a copy but that very variable you're getting. This is a common issue with foreach and delayed execution (multithreaded or not), since the foreach continues num is getting it's next value and if it does so because your thread gets to calculate, it's that value that will be calculated.

If you didn't multithread this but instead called the result of the lambda after the foreach what you would see would be 3 3 3 3 instead, in this case you're simply seeing them ofset by one because, most likely, the time it takes to start the thread is about the same as 1 iteration.

When you're making a copy of the variable that variable is declared within the scope of the foreach and it's a new variable each time, it's not getting changed to the next member and that variable is the one getting captured, giving you the correct result. This is the correct way to do this. The result you're getting isn't unexpected but not guaranteed either, you could be getting anything from 0 1 2 3 to 3 3 3 3 with the 1st method, the second method guarantees a correct output.

C# simple multithreading - why is it not producing the correct results?

1 Answers1