appccelerate / statemachine

A .net library that lets you build state machines (hierarchical, async with fluent definition syntax and reporting capabilities).
Apache License 2.0
496 stars 129 forks source link

ActiveStateMachine may deadlock when used with async/await #40

Closed estrizhok closed 4 years ago

estrizhok commented 5 years ago

I am not sure whether it is an actual bug, or I simply do something stupid. But consider the following snippet:

  internal class BuggyStateMachine : ActiveStateMachine<int, int>
  {
    private TaskCompletionSource<int> myTcs = new TaskCompletionSource<int>();

    public BuggyStateMachine()
    {
      In(0).On(1).Execute(() => myTcs.SetResult(0));
      Initialize(0);
      Fire(1);
    }

    public async Task RunDeadlock()
    {
      Start();
      await myTcs.Task;
      Stop();
    }

    public Task RunNoDeadlock()
    {
      Start();
      return myTcs.Task.ContinueWith(t => Stop());
    }
  }

A task returned by RunDeadlock will never finish when the method is executed on a thread without SynchronizationContext. This is because the continuation Stop() will be run on the same thread as () => myTcs.SetResult(0), which is a worker thread of the state machine. And since Stop is a synchronous method waiting for the state machine to finish, we have a deadlock.

ursenzler commented 5 years ago

Hi,

I have to look deeper into this, the Stop and Start methods are tricky ones.

Thanks for the issue.

Happy coding, Urs

ursenzler commented 5 years ago

You should probably add .ConfigureAwait(false) on the call to await myTcs.Task. but this depends on the target framework you are running this code on.

estrizhok commented 5 years ago

No, The example above is specific to the case when RunDeadlock is executed from a thread without SynchronizationContext. Thus, .ConfigureAwait(false) or .ConfigureAwait(true) does not alter it behavior a bit, as there is no context to capture.

ursenzler commented 4 years ago

Finally found the time to check this behaviour. You should use the AsyncActiveStateMachine when working with async/await. This code works as expected:

namespace Appccelerate.StateMachine.Facts
{
    using System.Threading.Tasks;
    using Appccelerate.StateMachine.AsyncMachine;
    using Xunit;

    // https://github.com/appccelerate/statemachine/issues/40
    public class DeadlockRepro
    {
        private AsyncActiveStateMachine<int, int> machine;

        private TaskCompletionSource<int> myTcs = new TaskCompletionSource<int>();

        public DeadlockRepro()
        {
            var builder = new StateMachineDefinitionBuilder<int, int>();

            builder
                .In(0).On(1).Execute(() => myTcs.SetResult(0));

            machine = builder
                .WithInitialState(0)
                .Build()
                .CreateActiveStateMachine();
        }

        [Fact]
        public async Task DoesNotDeadlock()
        {
            await machine.Fire(1);
            await machine.Start();
            await myTcs.Task.ConfigureAwait(false);
            await machine.Stop();
        }
    }
}