Running async tasks and cancelling after a timeout if necessary

Question

I have a few tools that run automatically. In those tools I usually run two to six jobs that run asynchronically. Sometimes when there is more data then expected or to prevent them from running indefinitely in case of a bug I cancel them.

The following example demonstrates my pattern with 90% accuracy. This is, the only parts that are left out is the actual business logic, IoC and the names of the processors are generic. Everthing else is pretty much the same.

I was wondering how good or bad this pattern is? It's been working fine for many days but maybe there is something that I missed and will give me headaches someday?

---

I start the application from the Main where I initialize the processors and the Timer (from System.Threading). Then I wait untill they all are done or cancel the one that might be either hanging or working with too much data at once.

void Main()
{
    var processors = new Processor[]
    {
        new Processor1(),
        new Processor2(),
        new Processor3(),
    };

    var token = new CancellationTokenSource();
    var timer = new Timer(
        callback: state => token.Cancel(),
        state: null,
        dueTime: TimeSpan.FromSeconds(5),
        period: TimeSpan.FromMilliseconds(-1));

    try
    {
        Task.WaitAll(processors.Select(p => p.Start(token.Token)).ToArray());
    }
    catch (AggregateException ex)
    {
        Console.WriteLine(ex);
    }
    finally
    {
        // When I use a try/catch already, I usually don't use 'using' to avoid deep nesting
        timer.Dispose();
        token.Dispose();
    }
}

---

Each processor is derived from the Processor class that provides logging for unexpected errors and awaits the core method to return.

abstract class Processor
{
    public Task Start(CancellationToken cancellationToken)
    {
        try
        {
            return Task.Run(async () => await StartCore(cancellationToken));
        }
        catch (Exception ex)
        {
            // logging
            return Task.FromException(ex);
        }
    }

    protected abstract Task StartCore(CancellationToken cancellationToken);
}

---

The processors themselves just do some work.

class Processor1 : Processor
{
    protected override Task StartCore(CancellationToken cancellationToken)
    {
        Console.WriteLine(GetType().Name + " started.");
        for (int i = 0; i < 6; i++)
        {
            cancellationToken.ThrowIfCancellationRequested();
            Thread.Sleep(TimeSpan.FromSeconds(1));
        }
        Console.WriteLine(GetType().Name + " finished.");
        return Task.FromResult<object>(null);
    }
}

class Processor2 : Processor
{
    protected override Task StartCore(CancellationToken cancellationToken)
    {
        Console.WriteLine(GetType().Name + " started.");
        for (int i = 0; i < 2; i++)
        {
            cancellationToken.ThrowIfCancellationRequested();
            Thread.Sleep(TimeSpan.FromSeconds(1));
        }
        Console.WriteLine(GetType().Name + " finished.");
        return Task.FromResult<object>(null);
    }
}

class Processor3 : Processor
{
    protected override Task StartCore(CancellationToken cancellationToken)
    {
        Console.WriteLine(GetType().Name + " started.");
        for (int i = 0; i < 4; i++)
        {
            cancellationToken.ThrowIfCancellationRequested();
            Thread.Sleep(TimeSpan.FromSeconds(1));
        }
        Console.WriteLine(GetType().Name + " finished.");
        return Task.FromResult<object>(null);
    }
}

Answer 1

Can't you replace

protected abstract Task StartCore(CancellationToken cancellationToken);

with

protected abstract void StartCore(CancellationToken cancellationToken); 

and call it as

Task.Run(() => {
                  try 
                  {
                      StartCore(cancellationToken);
                  }
                  catch
                  {
                     //logging
                     throw;
                  }
                });

? Why do you need async/await?

---

You don't really need a timer, you can use WaitAll overload that takes TimeSpan:

var tasks = processors.Select(p => p.Start(token.Token)).ToArray();
if (!Task.WaitAll(tasks, TimeSpan.FromSeconds(5)))
{
    token.Cancel();
    // wait for cancellation, if necessary
    // Task.WaitAll(tasks); 
}

---

You don't show your business logic and you don't say what kind of processing you are doing, so it's hard to tell whether inheritance is the best approach. It doesn't look like it based on your examples. Maybe it is better to have a single Processor class that would contain all the boiler-plate code you require (similar to BackgroundWorker) and inject StartCore as delegate/interface instead.