CODEWITHSUNDEEP
c#multithreadingloopsclosures
QueueHammer
QueueHammer

Reputation: 10824

Odd (loop / thread / string / lambda) behavior in C#

I have a snippet of code that I thought would work because of closures; however, the result proves otherwise. What is going on here for it to not produce the expected output (one of each word)?

Code:

string[] source = new string[] {"this", "that", "other"};
List<Thread> testThreads = new List<Thread>();
foreach (string text in source)
{
    testThreads.Add(new Thread(() =>
    {
        Console.WriteLine(text);
    }));
}

testThreads.ForEach(t => t.Start())

Output:

other
other
other

Upvotes: 5

Views: 410

Answers (6)

Jon Skeet
Jon Skeet

Reputation: 1499770

This is a classic mistake of capturing a loop variable. This affects both for and foreach loops: assuming a typical construction, you have a single variable across the whole duration of the loop. When a variable is captured by a lambda expression or an anonymous method, it's the variable itself (not the value at the time of capture) which is captured. If you change the value of the variable and then execute the delegate, the delegate will "see" that change.

Eric Lippert goes into great detail on it in his blog: part 1, part 2.

The usual solution is to take a copy of the variable inside the loop:

string[] source = new string[] {"this", "that", "other"};
List<Thread> testThreads = new List<Thread>();
foreach (string text in source)
{
    string copy = text;
    testThreads.Add(new Thread(() =>
    {
        Console.WriteLine(copy);
    }));
}

testThreads.ForEach(t => t.Start())

The reason this works is that each delegate will now capture a different "instance" of the copy variable. The variable captured will be the one created for the iteration of the loop - which is assigned the value of text for that iteration. Lo and behold, it all works.

Upvotes: 4

David Ly
David Ly

Reputation: 31586

This has to do with the fact that closures capture the variable itself without evaluating it until it's actually used. After the end of the foreach loop the value of text is "other", and it as after the loop ends that the method is invoked, and at the time of invocation the value of the captured variable text is "other"

See this blog post from Eric Lippert for details. He explains the behavior and some of the reasons behind it.

Upvotes: 7

Justin Niessner
Justin Niessner

Reputation: 245389

Closures in C# don't capture the value of text at time of creation. Since the foreach loop finishes execution before any of the threads execute, the last value of text is given to each.

This can be remedied:

string[] source = new string[] {"this", "that", "other"};
List<Thread> testThreads = new List<Thread>();

foreach (string text in source)
{
    // Capture the text before using it in a closure
    string capturedText = text;

    testThreads.Add(new Thread(() =>
        {
            Console.WriteLine(capturedText);
        }));
}

testThreads.ForEach(t => t.Start());

As you can see, this code "captures" the value of text inside of each iteration of the for loop. This guarantees that the closure gets a unique reference for each iteration rather than sharing the same reference at the end.

Upvotes: 2

mihi
mihi

Reputation: 6735

Closures / lambdas cannot properly bind to foreach or loop counter variables. Copy the value to another local variable (not declared as a foreach or counter variable) and it will work as expected.

Upvotes: 0

Dan Tao
Dan Tao

Reputation: 128317

Others have explained why you're running into this problem.

Luckily, the fix is very easy:

foreach (string text in source)
{
    string textLocal = text; // this is all you need to add
    testThreads.Add(new Thread(() =>
    {
        Console.WriteLine(textLocal); // well, and change this line
    }));
}

Upvotes: 0

Darin Dimitrov
Darin Dimitrov

Reputation: 1038710

The reason this is happening is that by the moment you are starting your threads the loop has finished and the value of the text local variable is "other", so when you start the threads that's what gets printed. This could be easily fixed:

string[] source = new string[] {"this", "that", "other"};
foreach (string text in source)
{
    new Thread(t => Console.WriteLine(t)).Start(text);
}

Upvotes: 0

Related Questions