Single worker, multiple async callers in different threads

Question

My use case is this:

1. There is a long running process that I want to ensure is only ever running at most only a single time.
2. Multiple consumers from different threads may wait on the calculation.
3. If the process is not running, the first consumer triggers it.

Here is the code:

public class Worker {
    private int _working;
    private TaskCompletionSource? _tcs;

    public async Task DoWorkAsync() {
        var triggerWork = false;
        if (Interlocked.CompareExchange(ref _working, 1, 0) == 0) {
            triggerWork = true;
            _tcs = new TaskCompletionSource();
        }

        if (!triggerWork) {
            await _tcs!.Task;

            /*
             * Point of interest: previous worker might have just finished,
             * and a new worker might have started - but that is ok because
             * "DoWorkInternalAsync" is a transactioned operation.
             */

            return;
        }

        await DoWorkInternalAsync();
        _tcs!.SetResult();

        _working = 0;
    }

    private async Task DoWorkInternalAsync() {
        // Do work
    }
}

I would like a review for my code for efficiency and correctness, and also I'd be happy to hear suggestions on how to do it more succinctly (perhaps with the use of a library or some BCL class I might have missed)

Thanks

Answer 1

Your code looks ok with respect to correctness, but I have not tested it. One option to make it shorter is to cache the task directly. As you have Task (and not ValueTask) you may await it many times.

So more like this:

public class Worker
{
    private object _lock = new();
    private Task<int>? _task;

    public Task<int> DoWorkAsync()
    {
        return _task ?? SetTaskIfNull();

        Task<int> SetTaskIfNull()
        {
            lock (_lock)
                return _task ??= DoWorkInternalAsync();
        }
    }

    private async Task<int> DoWorkInternalAsync()
    {
        await Console.Out.WriteLineAsync("Start calc");
        await Task.Delay(1000);
        await Console.Out.WriteLineAsync("Calc done");
        return 10;
    }
}

You do not need to await on the child operation when you only want to return the task. This means you can use the lock statement and the code will be easier to read. The first null check avoid the lock, if the task is already there. The second null check prevents duplicate creation.

So it appears that you want to restart the calculation in case that there is only a completed one?

Then I'd suggest this:

public class Worker
{
    private object _lock = new();
    private Task<int>? _task;

    public Task<int> DoWorkAsync()
    {
        lock(_lock)
        {
            // null would mean that this thread is the first
            // true means we need to restart
            if ((_task?.IsCompleted) != false)
                _task = DoWorkInternalAsync(1);

            return _task;
        }
    }

    private async Task<int> DoWorkInternalAsync(int v)
    {
        await Console.Out.WriteLineAsync("Start calc");
        await Task.Delay(1000);
        await Console.Out.WriteLineAsync("Calc done");
        return v;
    }
}