Increment a variable when condition (LessThan a constant) is met in multithreading environment

Question

This is the context:

I need to run a method n times one after the other (not at the same time) and n can be incremented by multiple threads. I want to limit this to 255 times (condition) so I have next code:

public class MyClass
{
      int number = 0;

      public void Caller()
      {
         Thread thread = new Thread(new ThreadStart(Request));
         thread.Start();
         Thread thread2 = new Thread(new ThreadStart(Request));
         thread2.Start();
         Thread thread3 = new Thread(new ThreadStart(Request));
         thread3.Start();
      }

      public void Request()
      {
         // Condition
         if (number < 255)
         {
            // Queue a new method
            System.Threading.Interlocked.Increment(ref number);
         }

         // If it is the first time that Request is called, then it starts to run Method "number" times
         if (number == 1)
            StartRunnigMethods();
      }

      public void StartRunningMethods()
      {
         while (number > 0)
         {
            Method();
            System.Threading.Interlocked.Decrement(ref number);
         }
      }

      public void Method()
      {
        ...
      }
}

Due to the multithreading nature, I'm concerned about the Request method when I modify number if it is less than 255. So I implemented a solution but I'm not sure if it is a good implementation.

Modified code:

      public void Request()
      {
         InterlockedIncrementIfLessThan(ref number, 255);

         // It is the fisrt time Request is called
         if( number == 1)
            StartToRunTheMethod();
      }

      public bool InterlockedIncrementIfLessThan(ref int value, int condition)
      {
         int newValue, currentValue;

         do
         {
            int initialValue = value;
            currentValue = Thread.VolatileRead(ref value);
            if (currentValue >= condition) return false;
            newValue = initialValue + 1;
         }
         while (System.Threading.Interlocked.CompareExchange(ref value, newValue, initialValue) != initialValue);
         return true;
      }

What would be the best way to perform a comparison (less than) and if it is true then increment the variable (number)?

I'm new on these topics so may be you could recommend me good references to start with.

Answer 1

Generally it is not a good idea to combine Volatile and Interlocked operations. It is better to use either one or the other, and preferably the Interlocked class that has clearer semantics. You can achieve the functionality of Thread.VolatileRead by using the Interlocked.CompareExchange, performing a no-op that doesn't actually change the underlying value.

Also it is not a good idea to access directly a variable that is shared between threads. Lines like if (number < 255) or while (number > 0) are red flags (indicate possible bugs), and should be avoided.

Here are two methods you could use for conditional update of shared variables, using the Interlocked class: InterlockedUpdate and InterlockedAdd. Each method has two overloads, one that returns the current value and one that does not.

public delegate bool InterlockedUpdateDelegate<T>(T existingValue, out T newValue);

public static bool InterlockedUpdate(ref int location1,
    InterlockedUpdateDelegate<int> predicate, out int currentValue)
{
    int value1 = Interlocked.CompareExchange(ref location1, 0, 0);
    while (true)
    {
        bool updateApproved = predicate(value1, out int newValue);
        if (!updateApproved) { currentValue = value1; return false; }
        int value2 = Interlocked.CompareExchange(ref location1, newValue, value1);
        if (value2 == value1) { currentValue = newValue; return true; }
        value1 = value2;
    }
}

public static bool InterlockedUpdate(ref int location1,
    InterlockedUpdateDelegate<int> predicate)
    => InterlockedUpdate(ref location1, predicate, out _);

public static bool InterlockedAdd(ref int location1, int value,
    Func<int, bool> predicate, out int currentValue)
    => InterlockedUpdate(ref location1, (int existingValue, out int newValue) =>
    {
        newValue = existingValue + value;
        return predicate(existingValue);
    }, out currentValue);

public static bool InterlockedAdd(ref int location1, int value,
    Func<int, bool> predicate)
    => InterlockedAdd(ref location1, value, predicate, out _);

Usage example:

public void Request()
{
    bool incremented = InterlockedAdd(ref number, 1, v => v < 255,
        out var currentValue);

    if (incremented && currentValue == 1) StartRunningMethods();
}

public void StartRunningMethods()
{
    while (true)
    {
        bool decremented = InterlockedAdd(ref number, -1, v => v > 0);
        if (!decremented) break;
        Method();
    }
}

---

Update: The above implementation of InterlockedUpdate is pessimistic, because it assumes that the observed variable will be stale most of the time, and so it starts by requesting a fresh value. Here is an optimistic version of the same method, that assumes instead that the observed variable will be fresh most of the time. I haven't measure it, but I believe that the optimistic version should have better performance in low contention scenarios.

public static bool InterlockedUpdate(ref int location1,
    InterlockedUpdateDelegate<int> predicate, out int currentValue)
{
    int value1 = location1; // The value1 may be stale at this point
    bool isFresh = false;
    while (true)
    {
        bool updateApproved = predicate(value1, out int newValue);
        if (!updateApproved)
        {
            if (isFresh) { currentValue = value1; return false; }
            newValue = value1; // Try rewritting the possibly stale value
        }
        int value2 = Interlocked.CompareExchange(ref location1, newValue, value1);
        if (value2 == value1) { currentValue = newValue; return updateApproved; }
        value1 = value2;
        isFresh = true;
    }
}

Answer 2

If you don't insist on using Interlocked and your problem is not to run method in parallel and not to run it more than 255 times, as you stated in comments, I would suggest using SemaphoreSlim (or even just simple locking) with maxCount set to 1:

private static SemaphoreSlim semaphore = new SemaphoreSlim(1, 1);
private volatile int counter = 0; // to switch 

public void ShouldNotRunInParallelAndMoreThan255()
{
    if (counter >= 255)
    {
        throw new Exception("Limit reached"); // or just return
    }
    semaphore.Wait(); // or semaphore.WaitAsync() in case of async or long workloads
    try
    {
        if(counter >= 255)  
        {
            throw new Exception("Limit reached"); // or just return;            
        }
        counter++;
        // Do you stuff

    }
    finally
    {
        semaphore.Release();
    }
}