This code I saw by chance. The code is roughly as follows.
public class Question2
{
public void Run()
{
List<Task> tasks = new List<Task>();
for (int i = 0; i < 10000; i++)
{
tasks.Add(Task.Factory.StartNew(() =>
{
GetNextId();
}));
}
Task.WaitAll(tasks.ToArray());
Console.WriteLine("currentId=" + GetCurrentId());
Console.ReadKey();
}
private int currentId;
public int GetNextId()
{
currentId++;
return currentId;
}
public int GetCurrentId()
{
return currentId;
}
}
I'dont know why result is not 10000? Can somebody tell me why and how to correct it.
You just discoverd the topic of thread safety! currentId++ translates to
Now imagine your loop goes only until i < 3. The first thread is at step 1 and reads the value 0. Before it goes on to step 2, the second thread is at step 1 and also reads 0. Now both threas go to step 3, the will write both 1 into memory. Now the third thread starts and will read 1 from memory, will increment it to 2 and writes it into memory. Now you have 2 instead of the expected value 3.
To fix this, you can use Interlocked.Increment:
private int currentId;
public int GetNextId()
{
return Interlocked.Increment(ref currentId);
}
But beware, that depending on your use case, you might still get similar problems when you use the return value of GetNextId().
currentId++;
In a single threaded program, the sequence of events looks like this.
In your multi-threaded program, the sequence of events may look more like this.
So the the value of currentId has been incremented three times but the value is 1000.
One solution to this problem is to use the lock keyword. Using the lock keyword ensures that only one thread has access to a piece of code at one time.
private int currentId;
private object _object = new object();
public int GetNextId() {
lock (_object) {
currentId++;
return currentId;
}
}
The sequence of events now looks like this.
Note that the result will still be 9999 if the initial value is zero and you increment it 9999 times.