Proper way to implement a never ending task. (Timers vs Task)

Question

So, my app needs to perform an action almost continuously (with a pause of 10 seconds or so between each run) for as long as the app is running or a cancellation is requested. The work it needs to do has the possibility of taking up to 30 seconds.

Is it better to use a System.Timers.Timer and use AutoReset to make sure it doesn't perform the action before the previous "tick" has completed.

Or should I use a general Task in LongRunning mode with a cancellation token, and have a regular infinite while loop inside it calling the action doing the work with a 10 second Thread.Sleep between calls? As for the async/await model, I'm not sure it would be appropriate here as I don't have any return values from the work.

CancellationTokenSource wtoken;
Task task;

void StopWork()
{
    wtoken.Cancel();

    try 
    {
        task.Wait();
    } catch(AggregateException) { }
}

void StartWork()
{
    wtoken = new CancellationTokenSource();

    task = Task.Factory.StartNew(() =>
    {
        while (true)
        {
            wtoken.Token.ThrowIfCancellationRequested();
            DoWork();
            Thread.Sleep(10000);
        }
    }, wtoken, TaskCreationOptions.LongRunning);
}

void DoWork()
{
    // Some work that takes up to 30 seconds but isn't returning anything.
}

or just use a simple timer while using its AutoReset property, and call .Stop() to cancel it?

Solution 1

I'd use TPL Dataflow for this (since you're using .NET 4.5 and it uses Task internally). You can easily create an ActionBlock<TInput> which posts items to itself after it's processed it's action and waited an appropriate amount of time.

First, create a factory that will create your never-ending task:

ITargetBlock<DateTimeOffset> CreateNeverEndingTask(
    Action<DateTimeOffset> action, CancellationToken cancellationToken)
{
    // Validate parameters.
    if (action == null) throw new ArgumentNullException("action");

    // Declare the block variable, it needs to be captured.
    ActionBlock<DateTimeOffset> block = null;

    // Create the block, it will call itself, so
    // you need to separate the declaration and
    // the assignment.
    // Async so you can wait easily when the
    // delay comes.
    block = new ActionBlock<DateTimeOffset>(async now => {
        // Perform the action.
        action(now);

        // Wait.
        await Task.Delay(TimeSpan.FromSeconds(10), cancellationToken).
            // Doing this here because synchronization context more than
            // likely *doesn't* need to be captured for the continuation
            // here.  As a matter of fact, that would be downright
            // dangerous.
            ConfigureAwait(false);

        // Post the action back to the block.
        block.Post(DateTimeOffset.Now);
    }, new ExecutionDataflowBlockOptions { 
        CancellationToken = cancellationToken
    });

    // Return the block.
    return block;
}

I've chosen the ActionBlock<TInput> to take a DateTimeOffset structure; you have to pass a type parameter, and it might as well pass some useful state (you can change the nature of the state, if you want).

Also, note that the ActionBlock<TInput> by default processes only one item at a time, so you're guaranteed that only one action will be processed (meaning, you won't have to deal with reentrancy when it calls the Post extension method back on itself).

I've also passed the CancellationToken structure to both the constructor of the ActionBlock<TInput> and to the Task.Delay method call; if the process is cancelled, the cancellation will take place at the first possible opportunity.

From there, it's an easy refactoring of your code to store the ITargetBlock<DateTimeoffset> interface implemented by ActionBlock<TInput> (this is the higher-level abstraction representing blocks that are consumers, and you want to be able to trigger the consumption through a call to the Post extension method):

CancellationTokenSource wtoken;
ActionBlock<DateTimeOffset> task;

Your StartWork method:

void StartWork()
{
    // Create the token source.
    wtoken = new CancellationTokenSource();

    // Set the task.
    task = CreateNeverEndingTask(now => DoWork(), wtoken.Token);

    // Start the task.  Post the time.
    task.Post(DateTimeOffset.Now);
}

And then your StopWork method:

void StopWork()
{
    // CancellationTokenSource implements IDisposable.
    using (wtoken)
    {
        // Cancel.  This will cancel the task.
        wtoken.Cancel();
    }

    // Set everything to null, since the references
    // are on the class level and keeping them around
    // is holding onto invalid state.
    wtoken = null;
    task = null;
}

Why would you want to use TPL Dataflow here? A few reasons:

Separation of concerns

The CreateNeverEndingTask method is now a factory that creates your "service" so to speak. You control when it starts and stops, and it's completely self-contained. You don't have to interweave state control of the timer with other aspects of your code. You simply create the block, start it, and stop it when you're done.

More efficient use of threads/tasks/resources

The default scheduler for the blocks in TPL data flow is the same for a Task, which is the thread pool. By using the ActionBlock<TInput> to process your action, as well as a call to Task.Delay, you're yielding control of the thread that you were using when you're not actually doing anything. Granted, this actually leads to some overhead when you spawn up the new Task that will process the continuation, but that should be small, considering you aren't processing this in a tight loop (you're waiting ten seconds between invocations).

If the DoWork function actually can be made awaitable (namely, in that it returns a Task), then you can (possibly) optimize this even more by tweaking the factory method above to take a Func<DateTimeOffset, CancellationToken, Task> instead of an Action<DateTimeOffset>, like so:

ITargetBlock<DateTimeOffset> CreateNeverEndingTask(
    Func<DateTimeOffset, CancellationToken, Task> action, 
    CancellationToken cancellationToken)
{
    // Validate parameters.
    if (action == null) throw new ArgumentNullException("action");

    // Declare the block variable, it needs to be captured.
    ActionBlock<DateTimeOffset> block = null;

    // Create the block, it will call itself, so
    // you need to separate the declaration and
    // the assignment.
    // Async so you can wait easily when the
    // delay comes.
    block = new ActionBlock<DateTimeOffset>(async now => {
        // Perform the action.  Wait on the result.
        await action(now, cancellationToken).
            // Doing this here because synchronization context more than
            // likely *doesn't* need to be captured for the continuation
            // here.  As a matter of fact, that would be downright
            // dangerous.
            ConfigureAwait(false);

        // Wait.
        await Task.Delay(TimeSpan.FromSeconds(10), cancellationToken).
            // Same as above.
            ConfigureAwait(false);

        // Post the action back to the block.
        block.Post(DateTimeOffset.Now);
    }, new ExecutionDataflowBlockOptions { 
        CancellationToken = cancellationToken
    });

    // Return the block.
    return block;
}

Of course, it would be good practice to weave the CancellationToken through to your method (if it accepts one), which is done here.

That means you would then have a DoWorkAsync method with the following signature:

Task DoWorkAsync(CancellationToken cancellationToken);

You'd have to change (only slightly, and you're not bleeding out separation of concerns here) the StartWork method to account for the new signature passed to the CreateNeverEndingTask method, like so:

void StartWork()
{
    // Create the token source.
    wtoken = new CancellationTokenSource();

    // Set the task.
    task = CreateNeverEndingTask((now, ct) => DoWorkAsync(ct), wtoken.Token);

    // Start the task.  Post the time.
    task.Post(DateTimeOffset.Now, wtoken.Token);
}

Solution 2

I find the new Task-based interface to be very simple for doing things like this - even easier than using the Timer class.

There are some small adjustments you can make to your example. Instead of:

task = Task.Factory.StartNew(() =>
{
    while (true)
    {
        wtoken.Token.ThrowIfCancellationRequested();
        DoWork();
        Thread.Sleep(10000);
    }
}, wtoken, TaskCreationOptions.LongRunning);

You can do this:

task = Task.Run(async () =>  // <- marked async
{
    while (true)
    {
        DoWork();
        await Task.Delay(10000, wtoken.Token); // <- await with cancellation
    }
}, wtoken.Token);

This way the cancellation will happen instantaneously if inside the Task.Delay, rather than having to wait for the Thread.Sleep to finish.

Also, using Task.Delay over Thread.Sleep means you aren't tying up a thread doing nothing for the duration of the sleep.

If you're able, you can also make DoWork() accept a cancellation token, and the cancellation will be much more responsive.

Solution 3

Here is what I came up with:

• Inherit from NeverEndingTask and override the ExecutionCore method with the work you want to do.
• Changing ExecutionLoopDelayMs allows you to adjust the time between loops e.g. if you wanted to use a backoff algorithm.
• Start/Stop provide a synchronous interface to start/stop task.
• LongRunning means you will get one dedicated thread per NeverEndingTask.
• This class does not allocate memory in a loop unlike the ActionBlock based solution above.
• The code below is sketch, not necessarily production code :)

:

public abstract class NeverEndingTask
{
    // Using a CTS allows NeverEndingTask to "cancel itself"
    private readonly CancellationTokenSource _cts = new CancellationTokenSource();

    protected NeverEndingTask()
    {
         TheNeverEndingTask = new Task(
            () =>
            {
                // Wait to see if we get cancelled...
                while (!_cts.Token.WaitHandle.WaitOne(ExecutionLoopDelayMs))
                {
                    // Otherwise execute our code...
                    ExecutionCore(_cts.Token);
                }
                // If we were cancelled, use the idiomatic way to terminate task
                _cts.Token.ThrowIfCancellationRequested();
            },
            _cts.Token,
            TaskCreationOptions.DenyChildAttach | TaskCreationOptions.LongRunning);

        // Do not forget to observe faulted tasks - for NeverEndingTask faults are probably never desirable
        TheNeverEndingTask.ContinueWith(x =>
        {
            Trace.TraceError(x.Exception.InnerException.Message);
            // Log/Fire Events etc.
        }, TaskContinuationOptions.OnlyOnFaulted);

    }

    protected readonly int ExecutionLoopDelayMs = 0;
    protected Task TheNeverEndingTask;

    public void Start()
    {
       // Should throw if you try to start twice...
       TheNeverEndingTask.Start();
    }

    protected abstract void ExecutionCore(CancellationToken cancellationToken);

    public void Stop()
    {
        // This code should be reentrant...
        _cts.Cancel();
        TheNeverEndingTask.Wait();
    }
}

Comments

[Bovaz]

Hello, I am trying this implementation but I am facing issues. If my DoWork takes no argument, task = CreateNeverEndingTask(now => DoWork(), wtoken.Token); gives me a build error (type mismatch). On the other hand, if my DoWork takes a DateTimeOffset parameter, that same line gives me a different build error, telling me that no overload for DoWork takes 0 arguments. Would you please help me figure this one out?

[Bovaz]

Actually, I solved my issue by adding a cast to the line where I assign task and passing the parameter to DoWork: task = (ActionBlock<DateTimeOffset>)CreateNeverEndingTask(now => DoWork(now), wtoken.Token);

[Lukas Pirkl]

Whatch out what task you will get if you use the async lambda as parameter of Task.Factory.StartNew - blogs.msdn.com/b/pfxteam/archive/2011/10/24/10229468.aspx When you do task.Wait(); after cancel is requested, you will be waiting to incorrect task.

[porges]

Yes, this should actually be Task.Run now, which has the correct overload.

[Jeff]

According to http://blogs.msdn.com/b/pfxteam/archive/2011/10/24/10229468.‌​aspx it looks like Task.Run uses the thread pool, so your example using Task.Run instead of Task.Factory.StartNew with TaskCreationOptions.LongRunning doesn't do exactly the same thing - if I needed the task to use the LongRunning option, would I not be able to use Task.Run like you've shown, or am I missing something?

[porges]

@Lumirris: The point of async/await is to avoid tying up a thread for the whole time it's executing (here, during the Delay call the task is not using a thread). So using LongRunning is kind of incompatible with the goal of not tying up threads. If you want to guarantee running on its own thread, you can use it, but here you're going to be starting a thread that is sleeping most of the time. What's the use case?

[Jeff]

@Porges Point taken. My use case would be a task running an infinite loop, in which each iteration would do a chunk of work, and 'relax' for 2 seconds before doing another chuck of work on the next iteration. It's running forever, but taking regular 2 second breaks. My comment, though, was more about whether you could specify it being LongRunning using the Task.Run syntax. From the documentation, it looks like Task.Run is cleaner syntax, as long as you're happy with the default settings it uses. There doesn't appear to be an overload with it that takes a TaskCreationOptions argument.

[porges]

@Lumirris: you're right, there's no way to specify this; Task.Run (referencesource.microsoft.com/#mscorlib/system/threading/Ta‌​sks/…) is essentially the same as Task.Factory.StartNew (referencesource.microsoft.com/#mscorlib/system/threading/Ta‌​sks/…) with the default options. (But it does specify DenyChildAttach.)

[XOR]

You could also have change the type of "ActionBlock<DateTimeOffset> task;" to ITargetBlock<DateTimeOffset> task;

[Nate Gardner]

I believe this is likely to allocate memory forever, thus eventually leading to an overflow.

[casperOne]

@NateGardner In which part?

[Harry]

@porges: Doesn't any timer uses a thread that is sleeping most of the time? I thought it's how they are implemented. I understand the main question is what is better - a timer or handling tasks manually. For me using tasks seems a little simpler from code readability standpoint, but I'm not sure about the performance / cost. But since timers use tasks or threads internally, then you have event propagation code in between, I'd guess a task could be even cheaper than a timer, but of course less simple in some cases.