How to implement my own locking mechanism

Question

For an assignment (it's for concurrency if you wonder) - I have to implement my own lock

(more specific: a TaS, a TTas and an Array-Lock, as described in "The Art of Multiprocessor Programming")

There are some test in- and output schemes online that I tried (too bad they take pretty long to try).

Your program is to count 9-digit numbers that pass a certain test

(it's called the elfproef in dutch, I don't know the english equivalence, sorry).

Sometimes I got a slightly different number, which suggests that my lock doesn't work a 100% right.

I have implemented the locks like this:

interface Lock
{
    void Lock();
    void Unlock();
}

class TaSLock : Lock
{
    AtomicBool state = new AtomicBool(false);

    void Lock.Lock()
    { while (state.getAndSet(true)); }

    void Lock.Unlock()
    { state.getAndSet(false); }
}

The AtomicBool is implemented with an integer, because the Interlocked class doesn't have operations for Boolean variables. This isn't optimal in terms of memory usage but it doesn't (or shouldn't) matter for the speed.

class AtomicBool
{
    int value;
    static int True = 1, False = -1;

    public AtomicBool(bool value)
    {
        if (value) this.value = True;
        else this.value = False;
    }

    public void set(bool newValue)
    {
        if (newValue) Interlocked.Exchange(ref value, True);
        else Interlocked.Exchange(ref value, False);
    }

    public bool getAndSet(bool newValue)
    {
        int oldValue;
        if (newValue) oldValue = Interlocked.Exchange(ref value, True);
        else oldValue = Interlocked.Exchange(ref value, False);
        return (oldValue == True);
    }

    public bool get()
    {
        return (Interlocked.Add(ref value, 0) == 1);
    }
}

Now in the parallel part I have just used:

theLock.Lock();
counter++;
theLock.Unlock();

But each time I get slightly different results.

Is there something obvious I'm doing wrong?

Answer 1

Hans is right. Your atomic get-and-set boolean appears to be correct -- in fact, it appears to me to be somewhat over-engineered. And the lock appears to be correct as well, insofar as you've built yourself a potentially highly inefficient "spin lock". (That is, all the waiting threads just sit there in a tight loop asking "can I go yet? can I go yet?" instead of going to sleep until it is their turn.)

What is not correct is that your lock provides no guarantee whatsoever that any two threads that both have a view of "counter" have a consistent view of "counter". Two threads could be on different processors, and those different processors could have different copies of "counter" in their caches. The cached copies will be updated, and only occasionally written back to main memory, thereby effectively "losing" some increases.

The real implementation of locking in C# ensures that a full-fence memory barrier is imposed so that reads and writes cannot move "forwards and backwards in time" across the fence. That gives a hint to the processors that they need to not be so smart about caching "counter" so aggressively.