CountdownLatch Thread-Safety Check

I've written this class as an exercise for a synchronization construct to be used across multiple threads.

The intent is to have worker threads Increment() it, do some work, then finally Decrement()it.

Meanwhile, when a master thread decides to shut down the program, it signals the threads using a CancellationToken and Wait()s on this structure until all threads have ceased (signalled by _number hitting zero).

I am depending mainly on the fact that Interlocked operations imply full memory fences. I am concerned with the read on the Wait() method, see the comment there.

```
public class CountdownLatch
{
int _number;
readonly ManualResetEventSlim _event;

public CountdownLatch()
{
this._event = new ManualResetEventSlim();
}

public void Increment()
{
Interlocked.Increment(ref this._number);
}

public void Decrement()
{
int currentNumber;
bool firstAttempt = true;

do
{
currentNumber = this._number;
if (!firstAttempt)
{
var spinWait = new SpinWait();
spinWait.SpinOnce();
}
else
{
firstAttempt = false;
}

if (currentNumber == 0)
{
throw new InvalidOperationException("Attempt to decrement past zero");
}
} while (Interlocked.CompareExchange(ref this._number, currentNumber - 1, currentNumber) != currentNumber);

if (currentNumber == 1)
{
this._event.Set();
}
}

public bool Wait(TimeSpan timeout)
{
// Should this be a volatile read: Volatile.Read(ref this._number) ?
// AFAICT the memory barrier here would prevent instruction reordering
// when the order of reads/writes matter, but here it doesn't. I just care
// to have the latest value of this._number across all CPUs/cores.
// Or does volatile re

The Good

I don't write multithreaded code very often, so other reviewers will probably have other comments (read: I'm a multithreading beginner myself), but I like what I'm seeing.

Interlocked.Increment(ref this._number);

I don't see anything that could go wrong with this. This is how I've read a thread-safe increment should be done, using the Interlocked class.

while (Interlocked.CompareExchange(ref this._number, currentNumber - 1, currentNumber) != currentNumber)

Looks good to me. currentNumber belongs to the thread that's running this, so no thread can mess with that currentNumber while currentNumber - 1 or CompareExchange is being evaluated, and again the Interlocked class at play has my full trust.

The Bad

Ignorance is bliss, nothing in sight.

The Ugly

The very first thing that jumped at me, is the abundance of redundant this qualifiers. They're not needed, you have underscores:

int _number;
readonly ManualResetEventSlim _event;

I like the underscore prefix, because it allows me to have a _number field and a number parameter or local variable, for example:

public CountdownLatch(ManualResetEventSlim event)
{
    _event = event;
}

The way I read currentNumber, "current" stands for "current thread's working copy of _number". I find the name number conveys that more succinctly.

StackExchange Code Review Q#46407, answer score: 2