tsJensen

A quest for software excellence...

.NET Task Factory and Dangers of Iteration

I had a simple small array of objects. I wanted to send each of them into a method that would fire off and manage a long running process on each of them in a new thread pool thread.

So I remebered the coolness of the new .NET 4.0 Task and it's attendant Factory. Seemed simple enough but I quickly learned a lesson I should have already known.

I've illustrated in code my first two failures and the loop that finally got it right. Let me know what you think. Undoubtedly there is even a better way.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace TaskFactoryExample
{
  public class ServerManager
  {
    IServerLib[] serverLibs;
    List<Task> taskList = new List<Task>();
    List<CancellationTokenSource> cancelTokens = new List<CancellationTokenSource>();

    public ServerManager(IServerLib[] servers)
    {
      serverLibs = servers;
    }

    // FIRST FAIL: only the last lib in the iteration gets sent to all ManageStart calls
    internal void Start_FirstFailed(string[] args)
    {
      foreach (var lib in serverLibs)
      {
        var tokenSource = new CancellationTokenSource();
        taskList.Add(Task.Factory.StartNew(() => { ManageStart(lib, args); }, tokenSource.Token));
        cancelTokens.Add(tokenSource);
      }
    }

    // SECOND FAIL: i is incremented finally to serverLibs.Length before ManageStart is called
	// resulting in an index out of range exception
    internal void Start_SecondFailed(string[] args)
    {
      for (int i = 0; i < serverLibs.Length; i++)
      {
        var tokenSource = new CancellationTokenSource();
        taskList.Add(Task.Factory.StartNew(() => { ManageStart(serverLibs[i], args); }, tokenSource.Token));
        cancelTokens.Add(tokenSource);
      }
    }

    // finally got it right - get a local reference to the item in the array so ManageStart 
    // is fed the correct serverLib object
    internal void Start(string[] args)
    {
      for (int i = 0; i < serverLibs.Length; i++ )
      {
        var serverLib = serverLibs[i];
        var tokenSource = new CancellationTokenSource();
        taskList.Add(Task.Factory.StartNew(() => { ManageStart(serverLib, args); }, tokenSource.Token));
        cancelTokens.Add(tokenSource);
      }
    }

    private void ManageStart(IServerLib lib, string[] args)
    {
      try
      {
        //code redacted for brevity
        //start long running or ongoing process with lib on threadpool thread
        lib.Start();
      }
      catch (Exception e)
      {
        //TODO: log general exception catcher
        throw; //leave in for testing
      }
    }

    internal void Stop()
    {
      try
      {
        foreach (var lib in serverLibs) lib.Stop();
        foreach (var tokenSource in cancelTokens) tokenSource.Cancel();
        foreach (var t in taskList) if (t.IsCompleted) t.Dispose();
      }
      catch (Exception e)
      {
        //TODO: log general exception catcher
        throw; //leave in for testing
      }
    }
  }
}