CODEWITHSUNDEEP
c#multithreading
ThomasVestergaard
ThomasVestergaard

Reputation: 364

Keeping track of task number in multi threaded application

I'm writing a multi threaded application, where one part of the code must be thread safe to keep track of task numbers.

I have this method:

private void IncrementTaskNumber() {
   Interlocked.Increment(ref _TaskNumber);
}

_TaskNumber is a private int in the same class. The problem is, this throws me a "A property, indexer or dynamic member access may not be passed as an out or ref parameter" exception. To get around this, I do:

private void IncrementTaskNumber() {
   int _taskNum = _TaskNumber;
   Interlocked.Increment(ref _taskNum);
   _TaskNumber = _taskNum;
}

Is this still thread safe?

Upvotes: 1

Views: 223

Answers (3)

Tudor
Tudor

Reputation: 62469

This is definitely not thread-safe:

private void IncrementTaskNumber() {
   int _taskNum = _TaskNumber;
   Interlocked.Increment(ref _taskNum);
   _TaskNumber = _taskNum;
}

since a thread executing the sequence could be interrupted between each of these 3 operations. This won't work unless you introduce a loop that keeps retrying to increment if the local value changed in the meantime, but that would basically mean you are reimplementing Interlocked.Increment using Interlocked.Increment. :)

Just make _TaskNumber a member variable, not a property.

Upvotes: 0

Tipx
Tipx

Reputation: 7515

You don't seem to have a locking mecanism in the IncrementTaskNumber method. Unless you call it only from one place, it is not thread safe. The first implementation you wanted to do, even if it would have worked wouldn't have been either since Interlocked.Increment(ref _TaskNumber); might have been called a second time before the first would have been done, and writen to the ref parameter.

Edit: If you want to make it threadsafe, you could modify yoru method like this :

private void IncrementTaskNumber()
{
    lock (_TaskNumber)
        _TaskNumber++;
}

Edit 2: (You might want to look into other solutions if using lock is too costy for your application.)

Upvotes: 1

Reed Copsey
Reed Copsey

Reputation: 564861

_TaskNumber is a private int in the same class.

_TaskNumber has to be a private field for this to work. You likely have it as a private property.

Define it as:

private int _TaskNumber;

And it will work.

Also note that your current workaround introduces a race condition - you're effectively getting rid of the atomic increment by doing it using a temporary variable, which defeats the purpose of using Interlocked in the first place. You need to increment the field directly.

Upvotes: 3

Related Questions