Search code examples
c#eventsconcurrencytask-parallel-libraryrace-condition

Race condition from long running task firing update events


My goal is to implement a concurrent method using event based asynchronous programming (EBAP) model, to handle status changes on a viewmodel. I have a long running task that is queued from the UI thread, using a RunNextItem method on a viewmodel class that looks like the DemoViewModel class below.

public class DemoViewModel
{
    private DemoRunClass DemoRunner;
    private ItemViewModel _currentlyRunningItem;

    /// <summary>
    /// Execute long running task on another thread.
    /// </summary>
    public async Task RunNextItem()
    {
        // get viewmodel of next item and create a command for it.
        var nextRunnableItemViewModel = GetNextItem();
        var runItemCommand = GenerateCommand(nextRunnableItemViewModel);

        // store the current item in local variable
        _currentlyRunningItem = nextRunnableItemViewModel;

        try
        {
            await DemoRunner.ExecuteLongRunningTask(
                runItemCommand, Dispatcher.CurrentDispatcher).ConfigureAwait(false);
        }
        catch (Exception ex)
        { ... }
        finally
        {
            // clear local variable
            _currentlyRunningItem = null;
        }
    }

    /// <summary>
    /// Handle updates from DemoRunner class
    /// </summary>
    private void OnUpdateEvent(object status)
    {
        if (status == "Method1 complete")
            _curentlyRunningItem.OnMethod1Complete();
        // ...
        else if (status == "All Complete")
            _curentlyRunningItem.OnAllComplete();
    }
}

The class that implements the long running task looks like:

public class DemoRunClass
{
    public event EventHandler<object> ExecutionUpdate;
    public Dispatcher EventDispatcher;

    public Task ExecuteLongRunningTask(object command, Dispatcher dispatcher)
    {
        EventDispatcher = dispatcher; // store dispatcher from UI thread

        return Task.Factory.StartNew(() =>
        {
            Method1(); // call UpdateEvent(method 1 complete)
            Method2(); // call UpdateEvent(method 2 complete).. etc
            Method3();
            Method4();
            Method5();
            UpdateEvent("All Complete");
        });
    }

    public void UpdateEvent(object status)
    {
        if (!EventDispatcher.CheckAccess())
            EventDispatcher.Invoke(new Action<object>(UpdateEvent), status);
        else
        {
            ExecutionUpdate?.Invoke(null, status);
        }
    }

    private void Method1()
    {
        // do some work ..
        UpdateEvent("Method1 complete");
    }

    // ... 
}

The idea is that the DemoViewModel class stores a local instance of the ItemViewModel of the object that is to be "run". The execution of the "run" is handled in the DemoRunClass which simply executes a number of methods, each of which will raise an event to signal the progress/status of the task. When it is complete, it fires the same event again with a "Complete" status. The task is awaited by the DemoViewModel, after which it sets the _currentlyRunningItem field to null;

The event handler in DemoViewModel handles these status events and updates the _currentlyRunningItem viewmodel accordingly. The dispatcher from the UI thread is passed in as a parameter so the event updates can be handled back on the UI thread.

The problem is: there is a race condition. The event for "All Complete" is invoked on the UI thread dispatcher, but then immediately after the await yields control back to the calling method, the finally block is hit and sets the current item to null, and then the OnUpdateEvent event handler will throw an exception trying to use the null current item field.

The MSDN docs for Dispatcher.Invoke claims it "Executes the specified delegate synchronously on the thread the Dispatcher is associated with." This does not align with my experience above, where it seems to invoke the delegate asynchronously and return before my eventhandler method is called. I have also tried using SyncronisationContext.Send() instead of the dispatcher, with the exact same result.

I'm starting to pull hairs here. Either Ι'm completely misunderstanding the meaning of "synchronous" or Ι'm doing something horribly wrong. How can I fix this problem and ensure the program executes the way I want it to, or is there a better solution than this?


Solution

  • Answer: The race condition was solved by removing the ".ConfigureAwait(false)" from the RunNextItem() method. I don't have a complete understanding of what was happening under the hood, but it seems that this was allowing immediate continuation on a different thread than the event handlers which in turn caused the race condition.