Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is this a correct Interlocked synchronization design?

I have a system that takes Samples. I have multiple client threads in the application that are interested in these Samples, but the actual process of taking a Sample can only occur in one context. It's fast enough that it's okay for it to block the calling process until Sampling is done, but slow enough that I don't want multiple threads piling up requests. I came up with this design (stripped down to minimal details):

public class Sample
{
    private static Sample _lastSample;
    private static int _isSampling;

    public static Sample TakeSample(AutomationManager automation)
    {
        //Only start sampling if not already sampling in some other context
        if (Interlocked.CompareExchange(ref _isSampling, 0, 1) == 0)
        {
            try
            {
                Sample sample = new Sample();
                sample.PerformSampling(automation);
                _lastSample = sample;
            }
            finally
            {
                //We're done sampling
                _isSampling = 0;
            }
        }

        return _lastSample;
    }

    private void PerformSampling(AutomationManager automation)
    {
        //Lots of stuff going on that shouldn't be run in more than one context at the same time
    }
}

Is this safe for use in the scenario I described?

like image 886
Dan Bryant Avatar asked Oct 15 '22 07:10

Dan Bryant


1 Answers

Yes, it looks safe because int is an atomic type here. But I would still advice replacing

private static int _isSampling;

with

private static object _samplingLock = new object();

and use :

lock(_samplingLock)
{
    Sample sample = new Sample();
    sample.PerformSampling(automation);
   _lastSample = sample;
}

Simply because it is the recommended pattern and also makes sure all access to _lastSample is treated correctly.

NB: I would expect comparable speed, lock uses the managed Monitor class that uses Interlocked internally.

Edit:

I missed the back-off aspect, here is another version :

   if (System.Threading.Monitor.TryEnter(_samplingLock))
   {
     try
     {
         .... // sample stuff
     }
     finally
     {
          System.Threading.Monitor.Exit(_samplingLock);
     }
   }
like image 73
Henk Holterman Avatar answered Oct 19 '22 15:10

Henk Holterman