TPL inside Windows Service

I need to perform few tasks inside a Windows Service I am writing in parallel. I am using VS2013, .NET 4.5 and this post shows that TPL is the way to go.

I was wondering if anyone can tell me if I have done it correctly.

public partial class FtpLink : ServiceBase
{
    private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource();
    private readonly ManualResetEvent _runCompleteEvent = new ManualResetEvent(false);

    public FtpLink()
    {
        InitializeComponent();

        // Load configuration
        WebEnvironment.Instance.Initialise();
    }

    protected override void OnStart(string[] args)
    {
        Trace.TraceInformation("DatabaseToFtp is running");

        try
        {
            RunAsync(_cancellationTokenSource.Token).Wait();
        }
        finally
        {
            _runCompleteEvent.Set();
        }
    }

    protected override void OnStop()
    {
        Trace.TraceInformation("DatabaseToFtp is stopping");

        _cancellationTokenSource.Cancel();
        _runCompleteEvent.WaitOne();

        Trace.TraceInformation("DatabaseToFtp has stopped");
    }

    private async Task RunAsync(CancellationToken cancellationToken)
    {
        while (!cancellationToken.IsCancellationRequested)
        {
            Trace.TraceInformation("Working");

            // Do the actual work
            var tasks = new List
            {
                Task.Factory.StartNew(() => new Processor().ProcessMessageFiles(), cancellationToken),
                Task.Factory.StartNew(() => new Processor().ProcessFirmware(), cancellationToken)
            };

            Task.WaitAll(tasks.ToArray(), cancellationToken);

            // Delay the loop for a certain time
            await Task.Delay(WebEnvironment.Instance.DatabasePollInterval, cancellationToken);
        }
    }
}

The general design of your example is correct, however there are some problems with how you have implemented the async code. Most notibly

• Calling .Wait() on the Task returned by RunAsync() will block 'OnStart()'. Instead it should be done after cancellation in the OnStop() method. This would eliminate the need to use a ManualResetEvent

• Task.WaitAll() should be replace with await Task.WhenAll()

More importantly you can achieve the desired behaviour much more simply by using a System.Threading.Timer. The TPL is needed only to perform parallel processing.

public partial class FtpLink : ServiceBase
{
    private Timer _timer;

    public FtpLink()
    {
        WebEnvironment.Instance.Initialise();
    }

    protected override void OnStart(string[] args)
    {
        Trace.TraceInformation("DatabaseToFtp service started.");
        _timer = new Timer(Process, null, 0, WebEnvironment.Instance.DatabasePollInterval);
    }

    protected override void OnStop()
    {
        _timer.Dispose();
        Trace.TraceInformation("DatabaseToFtp service stopped.");
    }

    private void Process(object state)
    {
        Trace.TraceInformation("Processing message files and firmware...");

        Parallel.Invoke(
            () => new Processor().ProcessMessageFiles(),
            () => new Processor().ProcessFirmware());

        Trace.TraceInformation("Processing complete.");
    }
}

If you want to ensure that the service waits for processing to complete before exiting, you can use a simple Monitor as shown in the answer to this question.

StackExchange Code Review Q#86351, answer score: 5