Concurrent Task Waiter

Question

I have some code designed to simplify managing multiple asynchronous operations. The code creates callback actions that, when executed by the asynchronous operation, track which asynchronous methods have completed. When all have completed, the class executes a specially-assigned method.

The purpose of this is to wait for multiple resources to load, without chaining them, in projects that do not have access to async / await.

One note on style: The egregious use of regions and over-commenting is company style, I'm afraid, and I can't change it. Otherwise I'm looking for advice on style, efficiency, security, and particularly type safety.

/// <summary>
/// Waits for multiple concurrent operations to finish before firing a callback 
/// </summary>
public class ConcurrentSynchronizer
{

    #region Properties

    private readonly List<Action> callBacks = new List<Action>();
    private int checkCount;

    /// <summary>
    /// The callbacks that must all be executed before the finished function is fired.
    /// </summary>
    public IEnumerable<Action> CallBacks
    {
        get { return callBacks; }
    }

    #endregion Properties

    #region Private Fields

    /// <summary>
    /// The action to perform when all assigned asynchronous methods have completed 
    /// </summary>
    private Action finishedFunction;

    #endregion Private Fields

    #region Constructors

    /// <summary>
    /// Creates a new instance of the concurrent synchronizer
    /// </summary>
    /// <param name="finishedFunction">Method that is executed when all assigned asynchronous methods have completed.</param>
    public ConcurrentSynchronizer(Action finishedFunction)
    {
        this.finishedFunction = finishedFunction;
    }

    #endregion Constructors

    #region Public Methods

    /// <summary>
    /// Create a callback function to send to the asynchronous method
    /// </summary>
    /// <returns> A callback action that invokes an internal checking function </returns>
    public Action CreateCallBack()
    {
        return AssignAndReturn((Action)Check);
    }

    /// <summary>
    /// Create a callback function to send to the asynchronous method, combined with a provided function 
    /// </summary>
    /// <param name="additionalFunction"> An additional function to call on the method's completion </param>
    /// <returns>
    /// A callback action that invokes an internal checking function as well as an additional function
    /// </returns>
    public Action CreateCallBack(Action additionalFunction)
    {
        return AssignAndReturn((Action)Action.Combine((Action)Check, additionalFunction));
    }

    #endregion Public Methods

    #region Private Methods

    /// <summary>
    /// Add the action to the callbacks list and return a reference to the stored index 
    /// </summary>
    /// <param name="action"> The action to add </param>
    /// <returns> A reference to the action in the callbacks list </returns>
    private Action AssignAndReturn(Action action)
    {

        callBacks.Add(action);

        return callBacks[callBacks.Count - 1];
    }

    /// <summary>
    /// Action called when async methods complete 
    /// </summary>
    private void Check()
    {
        checkCount++;
        if (checkCount == callBacks.Count)
        {
            finishedFunction();
        }
    }

    #endregion Private Methods
}

Typical usage is as follows:

Action finishedFunction = LongProcessFinished;
var concurrentSynchronizer = new ConcurrentSynchronizer(finishedFunction);

BackgroundWorker longProcess1 = new BackgroundWorker();
var longProcess1Callback = concurrentSynchronizer.CreateCallback();
longProcess1.DoWork += (o,e) =>
{
    DoLongProcess();
};
longProcess1.RunWorkerCompleted += (o, e) => longProcess1Callback();

//With an additional callback
Action longProcess2AdditionalCallback = AdditionalCallback;
var longProcess2 = new BackgroundWorker();
Action longProcess2Callback = concurrentSynchronizer.CreateCallback(longProcess2AdditionalCallback);
longProcess2.DoWork += (o,e) =>
{
    DoLongProcess2();
};
longProcess2.RunWorkerCompleted += (o, e) => longProcess2Callback();

longProcess1.RunWorkerAsync();
longProcess2.RunWorkerAsync();

Answer 1

The first question that comes to mind is did you consider an implementation based on Task.WhenAll(Task[] tasks) or WaitHandle.WaitAll()? Not what you're asking for but I would definitely ask that during a code review.

The following method and comment reveal a fundamental misunderstanding about objects and references.

/// <summary>
/// Add the action to the callbacks list and return a reference to the stored index 
/// </summary>
/// <param name="action"> The action to add </param>
/// <returns> A reference to the action in the callbacks list </returns>
private Action AssignAndReturn(Action action)
{

    callBacks.Add(action);

    return callBacks[callBacks.Count - 1];
}

A reference to the stored index? That does not make sense. The code returns a reference to the last Action object in the list. (Which may or may not refer to the same object referenced by the action argument depending on if the list is modified by another thread)

I would remove the method and simply inline the callbacks.Add(action) statement within the two referencing methods.

Security

• There is no gaurantee that any given callback is invoked at most once, possibly leading to a premature invocation of the finishedFunction.
• Check is called before the additionalFunction which can result in the finishedFunction executing before the additionalFunction.