Worst (won't actually work)
Change the access modifier of counter
to public volatile
As other people have mentioned, this on it's own isn't actually safe at all. The point of volatile
is that multiple threads running on multiple CPU's can and will cache data and re-order instructions.
If it's not volatile
, and CPU A increments a value, then CPU B may not actually see that incremented value until some time later, which may cause problems.
If it is, this just ensures the 2 CPU's see the same data at the same time. It doesn't stop them at all from interleaving their reads and write operations which is the problem you are trying to avoid.
Second Best:
lock(this.locker) this.counter++;
This is safe to do (provided you remember to lock
everywhere else that you access this.counter
).
It prevents any other threads from executing any other code which is guarded by locker
.
Using locks also, prevents the multi-cpu reordering problems as above, which is great.
The problem is, locking is slow, and if you re-use the locker
in some other place which is not really related then you can end up blocking your other threads for no reason.
Best
Interlocked.Increment(ref this.counter);
This is safe, as it effectively does the read, increment, and write in 'one hit' which can't be interrupted. Because of this it won't affect any other code and you don't need to remember to lock elsewhere either. It's also very fast (as MSDN says, on modern CPU's this is often literally a single CPU instruction).
I'm not entirely sure however if it gets around other CPU's reordering things, or if you also need to combine volatile with the increment.
Footnote: What volatile is actually good for.
As volatile
doesn't prevent these kind of multithreading issues, what's it for? A good example is say you have 2 threads, one which always writes to a variable (say queueLength
), and one which always reads from that same variable.
If queueLength
is not volatile, thread A may write 5 times, but thread B may see those writes as being delayed (or even potentially in the wrong order).
A solution would be to lock, but you could also in this situation use volatile, which would ensure that thread B will always see the most up-to-date thing that thread A has written, which would be faster and result in cleaner code.