Matthew Weber
Matthew Weber

Reputation: 13

Concurrency passing value type to 'Thread.Start' does not work as expected

In my code below, the Id property of ThreadClass is not set deterministically as expected (ThreadArray[0]'s ThreadClass.Id = 0, ThreadArray[1]'s ThreadClass.Id = 1, etc).

If I debug and slow down the Thread.Start()'s, everything works as expected. But when the program runs at full-speed, I get all Id's = 4 (or similar). I can't lock i because it's not a reference variable. Clearly, I am encountering a race condition. What am I doing wrong?

Main.cs

for (int i = 0; i < ThreadCount; i++)
{
    ThreadArray[i] = new Thread(() =>
        {
            new ThreadClass(i);
        });
    ThreadArray[i].Start();
}

ThreadClass.cs

private int Id { get; set; }

public ThreadClass(int i) {
    Id = id;
    while(true)
    {
        Console.WriteLine("I am thread " + i");
        Thread.Sleep(5000);
    }
}

Expected output:

I am thread 0
I am thread 1
I am thread 2
I am thread 3
... 5 second wait ...
I am thread 0
I am thread 1
I am thread 2
I am thread 3

Actual output:

I am thread 4
I am thread 4
I am thread 4
I am thread 4
... 5 second wait ...
I am thread 4
I am thread 4
I am thread 4
I am thread 4

Note that at this point each instance of ThreadArray is initialized to a valid Thread object.

Upvotes: 1

Views: 107

Answers (2)

exacerbatedexpert
exacerbatedexpert

Reputation: 818

You're using a version of C# that doesn't close over a fresh copy of the i variable. By the time the threads are executed, i is at or near ThreadCount. Simply create a copy of the variable so the closure can capture that instead:

for (int i = 0; i < ThreadCount; i++) 
{  
    int temp = i;
    ThreadArray[i] = new Thread(() =>  
        {              
            new ThreadClass(temp); 
        });      
}  

By the way, the version of C# in VS 2012 (or .NET 4.5) fixes this. See http://msdn.microsoft.com/en-us/library/hh678682(v=vs.110).aspx

Upvotes: 2

ExcaliburVT
ExcaliburVT

Reputation: 146

Move your ThreadArray[i].Start() outside your for loop and simply set up all your classes before starting the threads. Even though you will have to loop twice, it should ensure that everything occurs in the order/state that you want it to.

--EDIT Darnit got to the answer just before I did-- :)

Upvotes: -1

Related Questions