Weird Threading with C#

Question

I've encounter a weird problem with C# threading.

This is my sample program using thread to "activate" the Print() function at each agent in the agentList.

class Program {

    static void Main(string[] args) {

        List<Agent> agentList = new List<Agent>();

        agentList.Add(new Agent("lion"));
        agentList.Add(new Agent("cat"));
        agentList.Add(new Agent("dog"));
        agentList.Add(new Agent("bird"));

        foreach (var agent in agentList) {
            new Thread(() => agent.Print()).Start();
        }

        Console.ReadLine();
    }
}

class Agent {
    public string Name { get; set; }

    public Agent(string name) {
        this.Name = name;
    }

    public void Print() {
        Console.WriteLine("Agent {0} is called", this.Name);
    }
}

And here is the result when I run the above program:

Agent cat is called
Agent dog is called
Agent bird is called
Agent bird is called

But what I expected is something contains all 4 agents like

Agent lion is called
Agent cat is called
Agent dog is called
Agent bird is called

The most amazing thing is that if I called the threads outside the foreach, it works!

class Program {
    static void Main(string[] args) {
        List<Agent> agentList = new List<Agent>();

        agentList.Add(new Agent("leecom"));
        agentList.Add(new Agent("huanlv"));
        agentList.Add(new Agent("peter"));
        agentList.Add(new Agent("steve"));

        new Thread(() => agentList[0].Print()).Start();
        new Thread(() => agentList[1].Print()).Start();
        new Thread(() => agentList[2].Print()).Start();
        new Thread(() => agentList[3].Print()).Start();


        Console.ReadLine();
    }
}

The result of the above code is exactly what I expected. So what's the problem here?

Answer 1

What you have there is a closure. You're closing on a variable inside a foreach loop. What is happening is that the variable is getting overwritten before your thread starts so you have two iterations with the same value.

The easy fix is to capture the value inside the foreach loop before using it:

foreach(var a in agentList)
{
    var agent = a;
    new Thread(() => agent.Print()).Start();
}

Answer 2

You are capturing agent in a closure, which can be problematic with multi-threading. Assign to a local variable first:

    foreach (var agent in agentList) {
        var temp = agent;
        new Thread(() => temp.Print()).Start();
    }