2
\$\begingroup\$

I want to call an Action every X cycles (in my case, to a transaction commit every 100 records), so I do:

public class CyclicActionCaller
{
    private int _counter;
    private int _cycleValue;
    private Action _methodToCall;

    public CyclicActionCaller()
    {
        _counter = 0;            
    }

    public CyclicActionCaller Every(int cycleValue)
    {
        _cycleValue = cycleValue;
        return this;
    }

    public CyclicActionCaller Call(Action methodToCall)
    {
        _methodToCall = methodToCall;
        return this;
    }

    public void PerformCall()
    {
        Interlocked.Increment(ref _counter);
        // IS THIS THREAD SAFE - SHOULD BE CALLED EVERY _counter % _cycleValue - EVEN IN MULTITHREADING ENVIRONMENT
        if (_counter % _cycleValue == 0)
        {
            _methodToCall();                
        }
    }
}

CyclicActionCaller cyclicActionCaller = new CyclicActionCaller();
using (ModelContainer container = new ModelContainer())
{
   cyclicActionCaller.Every(100).Call(() => { container.SaveChanges(); });

   while(...) { // Import Loop - assume multiple workers inside
       cyclicActionCaller.PerformCall(); // every worker calls this after inserting a row
   }
}

Is this function thread safe? I want that - even if multiple threads call PerformCall - that it is just called once every 100 calls.

\$\endgroup\$
0
3
\$\begingroup\$

No, it's not thread-safe. Between the call to Interlocked.Increment() and the if check, another thread could call Interlocked.Increment(). That means both of them could see that _counter is 101, which means a call to _methodToCall would be skipped.

But this is exactly why Interlocked.Increment() returns the incremented value. So the thread-safe version would be:

public void PerformCall()
{
    if (Interlocked.Increment(ref _counter) % _cycleValue == 0)
        _methodToCall();                
}

This is exactly why writing code like this is hard (although this is a simple situation, so it's not that hard here). It's much easier to write correct multi-threaded code, if you lock all shared values.

Also, if you do something like this, you need to be sure that inserting a row in your code is also thread-safe. And the same applies to SaveChanges(). For example, if you used something like ConcurrentQueue in a straightforward way, it would be possible for SaveChanges() to save more than 100 items at a time, because new items could have been inserted between the call to Interlocked.Increment() and the final dequeue from the queue. This may not be a problem for you, but you should think about that.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.