When dealing with such a simple counter, it's best to use Interlocked.Increment:
private static int s_Number = 0;
public static int GetNextNumber()
{
return Interlocked.Increment(ref s_Number);
}
This will ensure each thread will return a unique value (as long as the number doesn't overflow), and that no increments will be lost.
Since the original code can be broken down into these steps:
- Read the existing value of
s_Number
- Add 1 to it
- Store the new value into
s_Number
- Read
s_Number
- Return the read value
The scenarios that can occur are:
- Both threads execute step 1 before the rest, which means both threads will read the same existing value, increment by 1, ending up on the same value. Lost an increment
- The threads can execute step 1 through 3 without conflict, but end up executing step 4 after both threads have updated the variable, retrieving the same value. Skipped a number
For larger pieces of code, where more data needs to be accessed atomically, a lock statement is usually a better way:
private readonly object _SomeLock = new object();
...
lock (_SomeLock)
{
// only 1 thread allowed in here at any one time
// manipulate the data structures here
}
But for such a simple piece of code, where all you need to do is atomically increment a field and retrieve the new value, Interlocked.Increment is better, faster, less code.
There are other methods on the Interlocked class as well, they're really handy in the scenarios they handle.
More detailed explanation of lost an increment.
Let's assume that s_Number starts at 0 before the two threads execute:
Thread 1 Thread 2
Read s_Number = 0
Read s_Number = 0
Add 1 to s_Number, getting 1
Add 1 to s_Number, getting 1 (same as thread 1)
Store into s_Number (now 1)
Store into s_Number (now 1)
Read s_Number = 1
Read s_Number = 1
Return read value (1)
Return read value (1)
As you can see above, the final value of s_Number should've been 2, and one of the threads should've returned 1, the other 2. Instead the final value was 1, and both threads returned 1. You lost an increment here.
Detailed explanation of skipping a number
Thread 1 Thread 2
Read s_Number = 0
Add 1 to s_Number, getting 1
Store into s_Number (now 1)
Read s_Number = 1
Add 1 to s_Number, getting 2
Store into s_Number (now 2)
Read s_Number = 2
Read s_Number = 2
Return read value (2)
Return read value (2)
Here the final result of s_Number will be 2, which is correct, but one of the threads should've returned 1, instead they both returned 2.
Let's see how the original code looks on an IL level. I'll add the original code into the IL instructions with comments
// public static int GetNumber()
// {
GetNumber:
// s_Number++;
IL_0000: ldsfld UserQuery.s_Number // step 1: Read s_Number
IL_0005: ldc.i4.1 // step 2: Add 1 to it
IL_0006: add // (part of step 2)
IL_0007: stsfld UserQuery.s_Number // step 3: Store into s_Number
// return s_Number;
IL_000C: ldsfld UserQuery.s_Number // step 4: Read s_Number
IL_0011: ret // step 5: Return the read value
// }
Note, I used LINQPad to get the IL code above, enabling optimizations (small /o+ in the lower right corner), if you want to play with the code to see how it converts into IL, download LINQPad and feed it this program:
void Main() { } // Necessary for LINQPad/Compiler to be happy
private static int s_Number = 0;
public static int GetNumber()
{
s_Number++;
return s_Number;
}