CODEWITHSUNDEEP
c#multithreading
Hieu Nguyen
Hieu Nguyen

Reputation: 404

Weird Threading with C#

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?

Upvotes: 5

Views: 242

Answers (5)

ratneshsinghparihar
ratneshsinghparihar

Reputation: 301

It is because of var agent which could have same refrence with in threads. That's why your eariler approach is not recommended.

Upvotes: 0

Ricky Smith
Ricky Smith

Reputation: 2389

Without using some kind of thread sync (like a ManualResetEvent), you cannot guarantee the order that threads will be processed. If you want to perform multiple steps in order, I would suggest you bundle the work up, then perform all of it in a single background thread.

I like the BackgroundWorker object:

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"));

BackgroundWorker worker = new BackgroundWorker();
worker.DoWork += (sender, e) =>
{
    foreach (var item in agentList)
    {
        item.Print();
    }
};

worker.RunWorkerAsync();

Upvotes: -1

James Michael Hare
James Michael Hare

Reputation: 38397

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();
    }

Upvotes: 2

Fischermaen
Fischermaen

Reputation: 12458

Change your foreach loop like this:

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

copying the value to a local variable (which looks a little stupid) avoids the thread of using a variable that might change when it comes to run.

Upvotes: 1

Justin Niessner
Justin Niessner

Reputation: 245389

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();
}

Upvotes: 9

Related Questions