I'm obviously doing something wrong here. I always get different number running this code:
var counter = new ConcurrentCounter();
var count = counter.Count();
Console.WriteLine(count);
public class ConcurrentCounter
{
private int _id;
private int _lock;
private int _counter;
public int Count()
{
var threads = Enumerable.Range(0, 10).Select(i => new Thread(Worker)).ToArray();
foreach (var thread in threads) thread.Start();
foreach (var thread in threads) thread.Join();
return _counter;
}
private void Worker()
{
for (var i = 0; i < 1000000; i++)
{
while (true)
{
var id = Interlocked.Increment(ref _id);
var current = Volatile.Read(ref _lock);
//Begin a locked block
if (Interlocked.CompareExchange(ref _lock, id, current) == current)
{
var value = Volatile.Read(ref _counter);
Volatile.Write(ref _counter, value + 1);
//Release the lock
Volatile.Write(ref _lock, current);
break;
}
}
}
}
}
What is wrong here? How can I get a working lock using Interlocked.CompareExchange? I cannot use usual C# lock as it will be used for inter-process shared memory lock.
The problem with your code is that every time a CAS fails, you will retrieve the _lock
value again, so the next CAS will definitely succeed, leading to this useless comparison. You need to block the thread until you've released the lock.
This is actually a simple spin lock, you do not need additional _id
to implement the lock, you only need to define two values:
int lockValue = 1;
int freeValue = 0;
for (var i = 0; i < 1000000; i++)
{
//Begin a locked block
while (Interlocked.CompareExchange(ref _lock, lockValue, freeValue) != freeValue)
Thread.Sleep(0);
var value = Volatile.Read(ref _counter);
Volatile.Write(ref _counter, value + 1);
//Release the lock
Volatile.Write(ref _lock, freeValue);
}