Thread Safe Counter used by Recursive Call to Function

Question

While using a global integer named GlobalID that I use as a counter, the following code works fine:

MyFunction(myClass thisClass, Int ID)
{   
     ListOfMyClass.add(thisClass)

     If (thisClass.IsPropertyValueAboveZero)
     {
          ID = GlobalID;
     }

     GlobalID++;

     Foreach (class someClass in ListOfClasses)
     {
           MyClass newClass = new MyClass(GlobalID, ID)
           MyFunction(newClass, ID)  //Recursive Call
     }
 }

I'm basically using GlobalID as a incrementing counter for each time the function is called. The counter is assigned a starting position for the first time it executes. I'm using a global variable because I wanted to make sure the ID is accurately increased for each pass regardless of execution entering or leaving the recursive call. This function is called (the first time....) from a ForEach loop that assigns the start position of global variable

My goal is to use a Parallel.ForEach for the initial call instead of the regular For Each loop. My problem deals with the counter. I don't know how to manage that counter within multiple threads. If I pass it as a variable to the function, I believe I will have a inaccurate lower / used number leaving the recursive loop. The global variable ensures the next number is higher than the previous number. The thisClass.IsPropertyValueAboveZero is just a arbitrary means of describing action based on a conditional statement. It has no meaningful reference to the rest of the code.

If I have multiple threads that have different starting positions for their counter, how do I accomplish making this thread safe? The only way I see at the moment is manually writing multiple versions of the same function and counter and using TaskFactory

Answer 1

For thread-safe counting, use the Interlocked methods: System.Threading.Interlocked.Increment(ref globalID)

private static int globalID;

// Be very careful about using a property to expose the counter
// do not *use* this value directly, but can be useful for debugging
// instead use the return values from Increment, Decrement, etc.
public int UnsafeGlobalID { get { return globalID; } }

Note: you need to use a field for the ref. It is possible to expose the field through a property, but can be problematic in code. It is better to use Interlock methods such as Interlocked.CompareExchange or explicit lock statements around logic requiring the value in a synchronized manner. The return value for Increment, Decrement, and so on is usually what should be used inside logic.

My problem deals with the counter. I don't know how to manage that counter within multiple threads. If I pass it as a variable to the function, I believe I will have a inaccurate lower / used number leaving the recursive loop. The global variable ensures the next number is higher than the previous number.

If I have multiple threads that have different starting positions for their counter, how do I accomplish making this thread safe? The only way I see at the moment is manually writing multiple versions of the same function and counter and using TaskFactory

The recursive call to MyFunction(newClass, ID) will have the value of ID, but I'm not sure what If (thisClass.IsPropertyValueAboveZero) is supposed to do though. Is it to make sure you have a non-zero starting point? If so, it would be better to make sure it is non-zero before the initial call outside of this function.

Also the logic in the foreach loop doesn't make sense to me. In MyClass newClass = new MyClass(GlobalID, ID) the ID will be argument value, or if IsPropertyValueAboveZero is true it will be the current value of GlobalID. So ID will typically be less than GlobalID since GlobalID is incremented before the foreach loop. I think you are passing GlobalID when you do not need to.

// if IsPropertyValueAboveZero is intended to start above zero
// then you can just initialize the counter to 1
private static int globalID = 1;

public void MyFunction(myClass thisClass, int id)
{
    // ListOfMyClass and ListOfClasses are probably not thread-safe
    // and you may need to add locks around the Add and get a copy
    // of ListOfClasses before the foreach enumeration
    // You may want to look at https://stackoverflow.com/a/6601832/29762
    ListOfMyClass.Add(thisClass); 

    foreach (class someClass in ListOfClasses)
    {
        int newId = System.Threading.Interlocked.Increment(ref globalID);
        MyClass newClass = new MyClass(newId);
        MyFunction(newClass, newId);  //Recursive Call
    }
}

Answer 2

For thread-safe integer incrementing of a single counter, consider using Interlocked.Increment.

Store the variable in a static:

public static int Bob;

Then increment it in a static function:

public static int IncrementBob()
{
    return Interlocked.Increment(ref Bob);
}

Anytime you want to increment, call IncrementBob.

Answer 3

If you use variable just as a counter standard usage of Interlocked.Increment works (see C# Thread safe fast(est) counter). Since your code actually wants to use the value of the counter you must be very careful to get correct value as reading from the value must be protected at the same time.

Since you only need to read value when you increment it than Interlocked.Increment is still enough. Be sure to never check or use value or GlobalID directly while multiple threads can modify it:

var ID = someValue; 
var newValue = Interlocked.Increment(ref GlobalID);
if (thisClass.IsPropertyValueAboveZero)
{
      // you can't use GlobalID directly here because it could be already incremented more
      ID = newValue - 1; // note -1 to match your original code.

}

Otherwise code get much trickier - for example if you only sometimes want to increment the global value but use it for every call (which probably not your case as sample shows incrementing always using less often) than you must pass current value to your function separately. Also in this case value passed to function and actual counter will have nothing to do with each other. If use case is anything other than "use value only when incremented" you probably should review what you are trying to achieve - most likely you would need some other data structure. Note that using lock to just protect reads and writes to counter not going to solve thread safety as you can't predict value you are about to read (may be incremented multiple times by other threads).

Note: sample code in the post shows some unclear usage of some old ID value and latest counter value. If code actually reflects what you want to do separating "counting number of calls" from "object ID" would be useful.