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

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);}

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.

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();    }}