Thread safe id generation with Interlocked in c#

Question

I am trying to understand Interlocked in C# in thread synchronization.

public int MethodUsedByMultipleThreads()
{
    var id = CreateNextId();
    return id;
}

private long CreateNextId()
{
    long id = 0;
    Interlocked.Exchange(ref id , this._nextId);
    Interlocked.Increment(ref this._nextId);
    return id;
}

Is the line

Interlocked.Exchange(ref id , this._nextId);

redundant if I directly use

Interlocked.Increment(ref this._nextId);
return _nextId;

Will it serve the same purpose?

Answer 1

The line

Interlocked.Exchange(ref id, this._nextId);

is both redundant and incorrect. It is redundant because it is practically the same as:

id = this._nextId

...because the id is a local variable that is not shared with other threads. And it is incorrect because there is a race condition between incrementing the _nextId field and returning it from the method CreateNextId(). The correct implementation is:

private long CreateNextId()
{
    long id;
    id = Interlocked.Increment(ref this._nextId) - 1;
    return id;
}

...or simply:

private long CreateNextId() => Interlocked.Increment(ref this._nextId) - 1;

The method Interlocked.Increment increments the _nextId field, and returns the incremented value as an atomic operation.

The subtraction - 1 is there to preserve the semantics of your existing code, regarding the meaning of the field _nextId. It might be preferable to change its name to _lastId or _idSeed, so that the - 1 can be removed.