1

I wonder is there a better solution for this task. One have a function which called concurrently by some amount of threads, but if some thread is already executing the code the other threads should skip that part of code and wait until that thread finish the execution. Here is what I have for now:

int _flag = 0;
readonly ManualResetEventSlim Mre = new ManualResetEventSlim();

void Foo()
{
    if (Interlocked.CompareExchange(ref _flag, 1, 0) == 0)
    {
        Mre.Reset();
        try
        {
            // do stuff
        }
        finally
        {
            Mre.Set();
            Interlocked.Exchange(ref _flag, 0);
        }
    }
    else
    {
        Mre.Wait();
    }
}

What I want to achieve is faster execution, lower overhead and prettier look.

VMAtm
  • 27,943
  • 17
  • 79
  • 125
ilivit
  • 41
  • 1
  • 7
  • I'm curious, what scenario do you have that is usefull to Wait() at the end of the method? In other words why would that thread wait? – brakeroo Nov 02 '16 at 14:48
  • What does the method actually do? You could use `ActionBlock` to queue messages of type T for execution, one-by-one. Or a `TransformBlock` that works on one input at a time and sends an output to subsequent blocks – Panagiotis Kanavos Nov 02 '16 at 14:54
  • The best way to handle concurrency problems is to avoid raw threads altogether. .NET already contains classes that implement most concurrent scenarios. Most of the time you'll end up implementing what's already in the BCL – Panagiotis Kanavos Nov 02 '16 at 14:55
  • @Dragos for example, I have a cache with expiration timeout. The expired items are removed when someone is trying to interact with the cache, but only one thread should process them and other should wait until it over. – ilivit Nov 02 '16 at 15:17
  • @ilivit this would be a lot easier if the other threads accessed the cache through a read-only interface like [IReadDictionary](https://msdn.microsoft.com/en-us/library/hh136548(v=vs.110).aspx). Both `Dictionary<>` and `ConcurrentDictionary<>` implement this interface – Panagiotis Kanavos Nov 02 '16 at 15:39

4 Answers4

0

You could use a combination of an AutoResetEvent and a Barrier to do this.

You can use the AutoResetEvent to ensure that only one thread enters a "work" method.

The Barrier is used to ensure that all the threads wait until the one that entered the "work" method has returned from it.

Here's some sample code:

using System;
using System.Threading;
using System.Threading.Tasks;

namespace Demo
{
    class Program
    {
        const int TASK_COUNT = 3;
        static readonly Barrier barrier = new Barrier(TASK_COUNT);
        static readonly AutoResetEvent gate = new AutoResetEvent(true);

        static void Main()
        {
            Parallel.Invoke(task, task, task);
        }

        static void task()
        {
            while (true)
            {
                Console.WriteLine(Thread.CurrentThread.ManagedThreadId + " is waiting at the gate.");

                // This bool is just for test purposes to prevent the same thread from doing the
                // work every time!

                bool didWork = false; 

                if (gate.WaitOne(0))
                {
                    work();
                    didWork = true;
                    gate.Set();
                }

                Console.WriteLine(Thread.CurrentThread.ManagedThreadId + " is waiting at the barrier.");
                barrier.SignalAndWait();

                if (didWork)
                    Thread.Sleep(10); // Give a different thread a chance to get past the gate!
            }
        }

        static void work()
        {
            Console.WriteLine(Thread.CurrentThread.ManagedThreadId + " is entering work()");
            Thread.Sleep(3000);
            Console.WriteLine(Thread.CurrentThread.ManagedThreadId + " is leaving work()");
        }
    }
}

However, it might well be that the Task Parallel Library might have a better, higher-level solution. It's worth reading up on it a bit.

Matthew Watson
  • 104,400
  • 10
  • 158
  • 276
0

First of all, the waiting threads wouldn't do anything, they only wait, and after they get the signal from the event, they simply move out of the method, so you should add the while loop. After that, you can use the AutoResetEvent instead of manual one, as @MatthewWatson suggested. Also, you may consider SpinWait inside the loop, which is a lightweight solution.

Second, why use int, if this is definitely bool nature for the flag field?

Third, why not to use the simple locking, as @grrrrrrrrrrrrr suggested? This is exactly what are you doing here: forcing other threads to wait for one. If your code should write something by only one thread in a given time, but can read by multiple threads, you can use the ReaderWriterLockSlim object for such synchronization.

VMAtm
  • 27,943
  • 17
  • 79
  • 125
0

What I want to achieve is faster execution, lower overhead and prettier look.

faster execution

unless your "Do Stuff" is extremely fast this code shouldn't have any major overhead.

lower overhead

Again, Interlocked Exchange,/CompareExchange are very low overhead, as is manual reset event.

If your "Do Stuff" is really fast, e.g. moving a linked list head, then you can spin:

prettier look

Correct multi-threaded C# code rarely looks pretty when compared to correct single threaded C# code. The language idioms are just not there yet.

That said: If you have a really fast operation ("a few tens of cycles"), then you can spin: (although without knowing exactly what your code is doing, I can't say if this is correct).

  if (Interlocked.CompareExchange(ref _flag, 1, 0) == 0)
        {
            try
            {
                // do stuff that is very quick.
            }
            finally
            {
                Interlocked.Exchange(ref _flag, 0);
            }
        }
        else
        {
            SpinWait.SpinUntil(() => _flag == 0);
        }
Jason Hernandez
  • 2,907
  • 1
  • 19
  • 30
-1

The first thing that springs to mind is to change it to use a lock. This won't skip the code, but will cause each thread getting to it to pause while the first thread executes its stuff. This way the lock will also automatically get released in the case of an exception.

object syncer = new object();
void Foo()
{
    lock(syncer)
    {
        //Do stuff
    }
}
gmn
  • 4,199
  • 4
  • 24
  • 46