How to catch that Task.Delay() canceled and do a callback?

Question

Lets say I have a worker class.

public sealed class Worker : IDisposable
{
    private bool _isRunning;
    private CancellationTokenSource _cts;

    private readonly Action _action;
    private readonly int _millisecondsDelay;
    private readonly object _lock = new object();

    public Worker(Action action, int millisecondsDelay)
    {
        _action = action;
        _millisecondsDelay = millisecondsDelay = 5000;
    }

    public void Start()
    {
        lock (_lock)
        {
            if (!_isRunning)
            {
                _isRunning = true;
                Run();
            }
        }
    }

    public void Cancel()
    {
        lock (_lock)
        {
            if (_isRunning) _cts.Cancel();
        }
    }

    private void Run()
    {
        using (_cts) _cts = new CancellationTokenSource();
        Task.Run(async () => { await DoAsync(_cts.Token); });
    }

    private async Task DoAsync(CancellationToken cancellationToken)
    {
        while (!cancellationToken.IsCancellationRequested)
        {
            //Log.Message1("____REFRESHING STATUS____");
            _action();
            await Task.Delay(_millisecondsDelay, cancellationToken);
        }

        //this code is unreachable
        lock (_lock)
        {
            _isRunning = false;
        }
    }

    public void Dispose()
    {
        try
        {
            _cts?.Cancel();
        }
        finally
        {
            if (_cts != null)
            {
                _cts.Dispose();
                _cts = null;
            }
        }
    }
}

The problem is the code _isRunning = false; is unreachable. I mean more likely when a caller call Cancel method the worker will be awaiting Task.Delay. So how I can call smth(here it's _isRunning = false;) after my Task will be canceled ? In other words I need to be sure that my worker is not running(it's not the cancelled state)

Is this for ASP.NET, ASP.NET Core, WPF, or some other specific platform? Some of them provide specific means for background tasks/processes with start/stop functionality. — Fire Lancer, Apr 18 '19 at 13:40
[`DispatcherTimer`](https://learn.microsoft.com/en-us/dotnet/api/system.windows.threading.dispatchertimer?view=netframework-4.7.2) may be relevant, if you want to ever access the UI from a timer. — Fire Lancer, Apr 18 '19 at 14:11

score 3 · Accepted Answer · answered Apr 18 '19 at 11:36

3

To answer your literal question, you can use a finally block:

private async Task DoAsync(CancellationToken cancellationToken)
{
  try
  {
    while (!cancellationToken.IsCancellationRequested)
    {
      //Log.Message1("____REFRESHING STATUS____");
      _action();
      await Task.Delay(_millisecondsDelay, cancellationToken);
    }
  }
  finally
  {
    lock (_lock)
    {
      _isRunning = false;
    }
  }
}

But I have some concerns about this "worker" approach:

I'm not a huge fan of the fire-and-forget inside Run. I suspect you'll want to change that.
Mixing lock with asynchronous code can be problematic. You should be absolutely sure that this is what you really want to do.

It may be worthwhile stepping back and reconsidering what you are actually wanting to do with this code.

answered Apr 18 '19 at 11:36

Stephen Cleary

437,863
77
675
810

Ok. thanks, it works. Basically I just need to call a method periodically(once per second/minute) by the way what you can advice as a better approach for it? – isxaker Apr 18 '19 at 12:05
`_isRunning` here looks a lot like just checking the status of the `Task` object (which would also be waitable)? Also if that throws, the `Task` provides a means to see the failure while a boolean flag doesn't. – Fire Lancer Apr 18 '19 at 13:38
@isxaker: How about a timer? – Stephen Cleary Apr 18 '19 at 15:24
@StephenCleary timer, really ? timers are out of date in my opinion at least. I've found this post also https://stackoverflow.com/questions/13695499/proper-way-to-implement-a-never-ending-task-timers-vs-task – isxaker Apr 19 '19 at 09:40
@FireLancer actually this flag says is the union of two task(action and delay) are working or not. And my action kind of safe - `Action action = ()=>{try{}catch{}};` – isxaker Apr 19 '19 at 09:42
@isxaker How is it a different union though? If `_action()` or `Delay` throw, the flag is set, and the task is set to an exception, (but then discarded as the code is written). If `_action()` and `Delay` do not throw and the loop exits, the flag is set, and the task is completed. Either way the task is completed, and could check the status and/or wait that? – Fire Lancer Apr 19 '19 at 10:09
@isxaker: I wouldn't say timers are out of date. They are not the best choice for many scenarios, e.g., if you have a bunch of Rx code already, using `Interval` instead of a timer would be cleaner. There's other tech that *is* out of date, e.g., `BackgroundWorker`, because *everything* you can do with BGW can be done more cleanly with `Task.Run` and friends. But there's no such replacement for timers AFAIK. So use the appropriate tech for your scenario, and for the "call a method periodically" use case, a timer seems entirely appropriate to me. – Stephen Cleary Apr 19 '19 at 13:39

How to catch that Task.Delay() canceled and do a callback?

1 Answers1