1

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?

Pavel Tupitsyn
  • 8,393
  • 3
  • 22
  • 44
Azeeb
  • 81
  • 11
  • 2
    Generally, you want to use *one* atomic operation. It looks like you should just be using `Increment` on the shared variable and *using the return value from that operation* rather than going back to the variable again. You're defeating the point of atomic operations. – Damien_The_Unbeliever Feb 01 '23 at 12:40

2 Answers2

2

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.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
1
long CreateNextId() => Interlocked.Increment(ref this._nextId);

This is all you need to generate sequential IDs in a thread-safe manner.


Do not use _nextId directly in any way: return _nextId is a mistake and will yield unpredictable results in a multithreaded environment - the value might be incremented by other threads after your call to Increment in the current thread.

Pavel Tupitsyn
  • 8,393
  • 3
  • 22
  • 44