When outputting, it is found that currentId is not 10000 as expected. Why? How to correct it

user_10673569 0
Viewed 98

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.

thread-safety multithreading c#
2 Answers

You just discoverd the topic of thread safety! currentId++ translates to

  1. read the value of currentId from memory
  2. increment this value by one
  3. write this new value back to memory

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().

user_82485700 
currentId++;

In a single threaded program, the sequence of events looks like this.

  1. Thread reads value of currentId (say, 999).
  2. Thread increments value and writes to currentId. Value of currentId is now 1000.

In your multi-threaded program, the sequence of events may look more like this.

  1. Thread 1 reads value of currentId (say, 999).
  2. Thread 2 reads value of currentId (also, 999).
  3. Thread 3 reads value of currentId (also, 999).
  4. Thread 2 increments value and writes to currentId. Value of currentId is now 1000.
  5. Thread 3 increments value and writes to currentId. Value of currentId is now 1000.
  6. Thread 1 increments value and writes to currentId. Value of currentId is now 1000.

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.

  1. Thread 1 gets access to the lock(_object).
  2. Thread 1 reads value of currentId (say, 999).
  3. Thread 2 requests access to the lock(_object) but is forced to wait until thread 1 releases the lock.
  4. Thread 3 requests access to the lock(_object). Request is queued behind thread 2's request.
  5. Thread 1 increments value and writes to currentId. Value of currentId is now 1000.
  6. Thread 1 releases the lock(_object).
  7. Thread 2 gets access to the lock(_object).
  8. Thread 2 reads value of currentId (now, 1000).
  9. Thread 2 increments value and writes to currentId. Value of currentId is now 1001.
  10. Thread 2 releases the lock(_object).
  11. Thread 3 gets access to the lock(_object).
  12. Thread 3 reads value of currentId (now, 1001).
  13. Thread 3 increments value and writes to currentId. Value of currentId is now 1002.
  14. Thread 3 releases the lock(_object).

Note that the result will still be 9999 if the initial value is zero and you increment it 9999 times.

user_38170680
Related