1

I want to run a cleanup task that might run for several seconds. Multiple threads could call this task but I want to run this task only once. All other invokations should just skip.

The following is my current implementation but I cannot imagine there is not a better solution in the .net framework resulting in less lines of code.

    object taskLock;
    bool isRunning;

    void Task()
    {
        if (isRunning) return;

        try
        {
            lock (taskLock)
            {
                if (isRunning) return;
                isRunning = true;
            }
            // Perform the magic
        }
        finally
        {
            isRunning = false;
        }
    }
Jesús López
  • 8,338
  • 7
  • 40
  • 66
Ramon Smits
  • 2,482
  • 1
  • 18
  • 20
  • you could use a similar approach, but use a concurrent dictionary in place of the taskLock object. tbh -if it currently works, move on and break down the other items in your backlog – jim tollan Feb 23 '16 at 09:39
  • "I cannot imagine there is not a better solution" This is unorthodox problem, since by definition it is ridden with concurrency ambiguities. Depending on timing, the task might be run only once - or for every invoking thread. At the very least, it is inefficient. – Dummy00001 Feb 23 '16 at 11:24
  • Yes, it can be very inefficient but I do not see a better alternative to signal for a certain task to happen. An alternative could be to maybe use a reset event where the task just waits until another thread sets the event but I then have to manage an thread/task. – Ramon Smits Feb 23 '16 at 15:25

1 Answers1

2

Yes, there is a better solution. You can use Interlocked.CompareExchange, the code becomes simpler and lock-free:

class Worker
{
    private volatile int isRunning = 0;

    public void DoWork()
    {
        if (isRunning == 0 && Interlocked.CompareExchange(ref isRunning, 1, 0) == 0)
        {
            try
            {
                DoTheMagic();
            }
            finally
            {
                isRunning = 0;
            }
        }
    }

    private void DoTheMagic()
    {
        // do something interesting
    }
}

In this case Interlocked.CompareExchange does the following as an atomic operation (pseudo-code):

wasRunning = isRunning;
if isRunning = 0 then 
     isRunning = 1
end if
return wasRunning

From the MSDN documentation:

public static int CompareExchange(
    ref int location1,
    int value,
    int comparand
)

If comparand and the value in location1 are equal, then value is stored in location1. Otherwise, no operation is performed. The compare and exchange operations are performed as an atomic operation. The return value of CompareExchange is the original value in location1, whether or not the exchange takes place

Jesús López
  • 8,338
  • 7
  • 40
  • 66
  • I like this solution but isn't the CompareExchange slower then the `if (isRunning) return;` ? Maybe doing `if (isRunning == 1 || Interlocked.CompareExchange(ref isRunning, 1, 0) == 1) return;` might be faster when the task would more often be running then not – Ramon Smits Feb 23 '16 at 15:35
  • @RamonSmits, yes it would be faster, but it should be `if (isRunning == 0 && Interlocked.CompareExchange(ref isRunning, 1, 0) == 0)` and you would need to declare `isRunning` as volatile. On the other hand `Interlocked.CompareExchange` is very fast. I don't believe you would get an appreciable gain. – Jesús López Feb 23 '16 at 16:04
  • @RamonSmits. I edited my answer to include the optimization. Here you have a micro-benchmark that compares the optimization https://gist.github.com/jesuslpm/77b60126c8d787c7a76b – Jesús López Feb 23 '16 at 16:53
  • Why is the 'volatile' keyword needed? – Ramon Smits Mar 11 '16 at 14:07
  • @RamonSmits because it can be modified by another thread. It prevents compiler from performing some optimizations that might lead to wrong results. It is related to memory fences https://en.wikipedia.org/wiki/Memory_barrier – Jesús López Mar 14 '16 at 13:08
  • @RamonSmits you may want to read this thread http://stackoverflow.com/questions/72275/when-should-the-volatile-keyword-be-used-in-c – Jesús López Mar 14 '16 at 14:01