Progress reporting and Task continuation race conditions

Question

I have a LogWriter class that accepts a file path and a System.Windows.Forms.TextBox and then writes to both in a public void Write(string progress) method. It uses a StreamWriter with AutoFlush to append to the file and, therefore, it itself implements IDisposable to dispose of the stream. It just sets the Text property on the text box each time.

I have a method in another class that spawns processes and uses event handlers on the ErrorDataReceived and OutputDataReceived events to call Report() on the Progess instance.

In an async button click event handler on the Form I do the following:

using(LogWriter logger = new LogWriter(filePath, txtProgress))
{
    IProgress<string> progress = new Progress<string>(s => logger.Write(s));
    await Task.Run(() => this._C.RunProcess(progress));
} //disposal point

Before that, I implemented it using a ContinueWith in a sync handler:

LogWriter logger = new LogWriter(filePath, txtProgress);
Task t = Task.Run(() => this._C.RunProcess(progress));
t.ContinueWith((_) => { coreCurrentLogger.Dispose(); }, TaskScheduler.FromCurrentSynchronizationContext());

I understand that calling Progress.Report() uses SynchonizationContext.Post() to queue the lambda to be executed some time later. In this case, I believe the SynchonizationContext will be that of the UI Thread.

In both cases this code behaves as I expect. The TextBox and file both contain the final string written to the output stream by the spawned process.

Based on the above, is there a race condition in either implementation where the LogWriter instance could be disposed before all of the Process.Report() lambdas have been executed?

Is there a difference between the two implementations? Do I need a ConfigureAwait(true) on the first example?

Answer 1

There is indeed a race condition here. Progress Posts the handlers, rather than sending them, so the worker is permitted to continue execution before the progress reporting has actually taken place, meaning in theory it's possible for the entire operation to end with one or more progress reporting handlers running.

The Progress class itself provides no way to wait for all existing handlers to finish.

A solution is to wrap the Progress object in one that tracks the number of ongoing handlers and provides a Task to notify you when all ongoing handlers are complete.

public class TrackableProgress<T> : IProgress<T>
{
    private IProgress<T> nestedProgress;
    private int handlersRunning = 0;
    private TaskCompletionSource? taskCompletionSource;
    public object key = new object();

    public TrackableProgress(Action<T> action)
    {
        nestedProgress = new Progress<T>(Handler);

        void Handler(T t)
        {
            try
            {
                lock (key)
                    handlersRunning++;

                action(t);
            }
            finally
            {
                lock (key)
                {
                    handlersRunning--;
                    if (handlersRunning == 0)
                    {
                        taskCompletionSource?.TrySetResult();
                        taskCompletionSource = null;
                    }
                }
            }
        }
    }
    void IProgress<T>.Report(T value)
    {
        nestedProgress.Report(value);
    }
    public Task HandlersFinished()
    {
        Task? result = null;
        lock (key)
        {
            if (handlersRunning > 0)
            {
                taskCompletionSource ??= new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
                result = taskCompletionSource.Task;
            }
        }
        return result ?? Task.CompletedTask;
    }
}

This will allow you to write:

using(LogWriter logger = new LogWriter(filePath, txtProgress))
{
    TrackableProgress<string> progress = new TrackableProgress<string>(s => logger.Write(s));
    await Task.Run(() => this._C.RunProcess(progress));
    await progress.HandlersFinished();
}