Run background work, but ensure that this can only run once at any given time

Question

I want to kick off some background work, I do not have to keep any reference. Start and forget. However I want to make sure that this background work is only running once at any time given.

Assuming that StartBackgroundWork is only ever called from one thread, can you improve this solution? Do you see any problems with this?

private bool _busy;

private void StartBackgroundWork()
{
    if(_busy)
    {
        return;
    }
    _busy = true;
    Task.Factory.StartNew(() => {
        try
        {
            // Do work
        }
        finally
        {
            _busy = false;
        }
    }, TaskCreationOptions.LongRunning);
}

Should I avoid using a Taskin this scenario?

So if several of them are kicked off during a short period of time, you want all of them to execute, or cancel the concurrent ones that are not the first to start ? — Pac0, Aug 24 '17 at 15:58
Only the first executes. The next invocation should not do anything unless the previous one has finished. `StartBackgroundWork` is only ever called by one thread. — Dennis Kuypers, Aug 24 '17 at 15:59
What is the exact problem you are trying to solve? _PS. I didn't vote yet._ — Dmitry, Aug 24 '17 at 16:22

inquisitive · Answer 1 · 2017-08-24T16:49:05.953

One problem that can happen with this approach that if a StartBackgroundWork call is made very close in time to when the background task was just cleaning up, then:

_busy = false; -happens-before- if(_busy)

may not be preserved. So you may miss some triggers.

As long as StartBackgroundWork ...is only ever called from one thread... you will not have overlapping executions, but the moment this condition is violated, the safety goes. Do note that this code might be maintained in future, and someone may call that method without considering (or understanding) threading implications. This type of guarantee is especially difficult to make for web applications.

One solution as proposed by @GhostTW is to perform the operations on _busy under lock. Locking does have some performance impact but is clean to maintain.

A smartass way to achieve the same objective would be to declare the flag as volatile. This is super fast but the way it achieves is so, umm... twisted, that a slight misuse can cause more harm than good. If performance is not microsecond critical, then recommend going with lock.

Ideally a private method need not be threadsafe itself, but your business rules say something on what is allowed in parallel and what is not. That makes your method thread-aware. Making it thread-safe or not is your choice. Depending on how hard your business rule is, you might not have a choice. Questions to ask: (1) what if we do miss a trigger? Does a patient die? (2) What if the task does end up running twice in parallel? Does a patient die? (3) What if someone looks at my code? Will that question my consulting fees? (last one was a joke)

But that is a very...very very small timeframe. The StartBackgroundWorker method is internal to my library and called by (another) background worker. It is not required to be thread-safe. — Dennis Kuypers, Aug 24 '17 at 16:29
A timeframe is a timeframe. It being small or wide does not change anything. What really matters is the criticality of a mistake. If all you miss is a frame in an animation, please go ahead. If a patient dies, please stop! If something in between, make a judgement call. — inquisitive, Aug 24 '17 at 16:51

score 1 · Answer 2 · answered Aug 24 '17 at 16:22

1

you need lock to make sure there has no race condition happen.

private bool _busy;
private object locker = new object();

private void StartBackgroundWork()
{
    lock (this.locker)
    {
        if (_busy)
        {
            return;
        }
        _busy = true;
    }

    Task.Factory.StartNew(() =>
    {
        try
        {
            // Do work
        }
        finally
        {

            lock (this.locker)
                _busy = false;
        }
    }, TaskCreationOptions.LongRunning);
}

answered Aug 24 '17 at 16:22

GhostTW

140
8

bool operations are atomic, no need to lock. http://www.ecma-international.org/publications/standards/Ecma-335.htm Assuming that `StartBackgroundWork` is only ever called from one thread the lock is not required, right? – Dennis Kuypers Aug 24 '17 at 16:24
@DennisKuypers, bool being atomic means that you will always have a `True` or a `False`, never in `Tralse`. It does not say anything about how quickly updates propagate across threads. An update made in one thread may take time to get visible in another thread. – inquisitive Aug 24 '17 at 16:36
@inquisitive I read that it might happen that the values doesn't propagate at all. Does this apply to this code as well? Because then the locks wouldn't help either, right? Does marking the bool as `volatile` resolve the problem? – Dennis Kuypers Aug 24 '17 at 16:45

Run background work, but ensure that this can only run once at any given time

2 Answers2

Linked