Async Method Weaving: InvalidOperationException at runtime

[Dhairya-Sangoi]

Hey,

It seems there is a bug in weaving async methods.

Attribute:

[InterceptorOptions(AlwaysCreateNewInstance = true)]
    public class TestingAnnotation : Attribute, IMethodInterceptor
    {
        public void OnEnter(Type declaringType, object instance, MethodBase methodbase, object[] values)
        {

        }

        public bool OnException(Exception e)
        {
            return true;
        }

        public void OnExit()
        {

        }
    }

Annotated class method:

[TestingAnnotation]
        public async Task TestAsync()
        {
            try
            {
                await Task.Delay(1000);
            }
            catch (Exception)
            {
                await Task.Delay(1000);
                await Task.Delay(1000);
            }
        }

Decompiled Weaved Code:

private sealed class \u003CTestAsync\u003Ed__4 : IAsyncStateMachine
    {
      public int \u003C\u003E1__state;
      public AsyncTaskMethodBuilder \u003C\u003Et__builder;
      public TestingClass \u003C\u003E4__this;
      private object \u003C\u003Es__1;
      private int \u003C\u003Es__2;
      private TaskAwaiter \u003C\u003Eu__1;
      public IMethodInterceptor \u003CTestAsync\u003E_4yx3x2rx223or;

      public \u003CTestAsync\u003Ed__4()
      {
        base.\u002Ector();
      }

      void IAsyncStateMachine.MoveNext()
      {
label_0:
        int num1 = this.\u003C\u003E1__state;
        try
        {
          try
          {
            if (num1 != 0)
            {
              // ISSUE: method reference
              // ISSUE: type reference
              this.\u003CTestAsync\u003E_4yx3x2rx223or.OnEnter(typeof (TestingClass), (object) this.\u003C\u003E4__this, MethodBase.GetMethodFromHandle(__methodref (TestingClass.TestAsync), __typeref (TestingClass)), (object[]) null);
            }
            TaskAwaiter awaiter1;
            int num2;
            TaskAwaiter awaiter2;
            switch (num1)
            {
              case 0:
                goto label_0;
              case 1:
                awaiter1 = this.\u003C\u003Eu__1;
                this.\u003C\u003Eu__1 = new TaskAwaiter();
                this.\u003C\u003E1__state = num1 = -1;
                break;
              case 2:
                awaiter2 = this.\u003C\u003Eu__1;
                this.\u003C\u003Eu__1 = new TaskAwaiter();
                this.\u003C\u003E1__state = num1 = -1;
                goto label_18;
              default:
                this.\u003C\u003Es__2 = 0;
                try
                {
                  TaskAwaiter awaiter3;
                  if (num1 != 0)
                  {
                    awaiter3 = Task.Delay(1000).GetAwaiter();
                    if (!awaiter3.IsCompleted)
                    {
                      this.\u003C\u003E1__state = num1 = 0;
                      this.\u003C\u003Eu__1 = awaiter3;
                      TestingClass.\u003CTestAsync\u003Ed__4 stateMachine = this;
                      this.\u003C\u003Et__builder.AwaitUnsafeOnCompleted<TaskAwaiter, TestingClass.\u003CTestAsync\u003Ed__4>(ref awaiter3, ref stateMachine);
                      return;
                    }
                  }
                  else
                  {
                    awaiter3 = this.\u003C\u003Eu__1;
                    this.\u003C\u003Eu__1 = new TaskAwaiter();
                    this.\u003C\u003E1__state = num1 = -1;
                  }
                  awaiter3.GetResult();
                }
                catch (Exception ex)
                {
                  this.\u003C\u003Es__1 = (object) ex;
                  this.\u003C\u003Es__2 = 1;
                }
                if (this.\u003C\u003Es__2 == 1)
                {
                  awaiter1 = Task.Delay(1000).GetAwaiter();
                  if (!awaiter1.IsCompleted)
                  {
                    this.\u003C\u003E1__state = num2 = 1;
                    this.\u003C\u003Eu__1 = awaiter1;
                    TestingClass.\u003CTestAsync\u003Ed__4 stateMachine = this;
                    this.\u003C\u003Et__builder.AwaitUnsafeOnCompleted<TaskAwaiter, TestingClass.\u003CTestAsync\u003Ed__4>(ref awaiter1, ref stateMachine);
                    return;
                  }
                  break;
                }
                goto label_19;
            }
            awaiter1.GetResult();
            awaiter2 = Task.Delay(1000).GetAwaiter();
            if (!awaiter2.IsCompleted)
            {
              this.\u003C\u003E1__state = num2 = 2;
              this.\u003C\u003Eu__1 = awaiter2;
              TestingClass.\u003CTestAsync\u003Ed__4 stateMachine = this;
              this.\u003C\u003Et__builder.AwaitUnsafeOnCompleted<TaskAwaiter, TestingClass.\u003CTestAsync\u003Ed__4>(ref awaiter2, ref stateMachine);
              return;
            }
label_18:
            awaiter2.GetResult();
label_19:
            this.\u003C\u003Es__1 = (object) null;
            if (num1 != 0)
              ;
          }
          finally
          {
            this.\u003CTestAsync\u003E_4yx3x2rx223or.OnExit();
          }
        }
        catch (Exception ex)
        {
          if (this.\u003CTestAsync\u003E_4yx3x2rx223or.OnException(ex))
          {
            this.\u003C\u003E1__state = -2;
            this.\u003C\u003Et__builder.SetException(ex);
            return;
          }
        }
        this.\u003C\u003E1__state = -2;
        this.\u003C\u003Et__builder.SetResult();
      }

      [DebuggerHidden]
      void IAsyncStateMachine.SetStateMachine(IAsyncStateMachine stateMachine)
      {
      }
    }

As it can be seen, the handling for state 0:

switch (num1)
            {
              case 0:
                goto label_0;

results in an infinite loop, if I understand correctly. Because this jumps to the label_0, which again sets num1 with current state 0.

Also,

This instruction goto label_0; is only weaved if there are multiple awaits in the catch block.

[reflection-emit]

I will look into this tomorrow evening. I have to get some sleep now.

[Dhairya-Sangoi]

No worries. Thank you very much for the quick help :)

[Dhairya-Sangoi]

Hey, I was curious about how compiler creates state machine for async methods with try-catch. I looked on various forums but all I could get was how state machine is created for a normal async method. If you have any knowledge about this, could you please help me with it?

[reflection-emit]

What do you mean with how it is created? As in the process of generating it or more the outcome of the generation?

[Dhairya-Sangoi]

Outcome of the generation. As in what is created when an async method with try-catch is converted to a state machine. Comparing it with the state machine generated for async methods with no try-catch, there seems to be some difference. And I was unable to find exact pattern as to what would a generic async method containing try-catch blocks be converted into.

[reflection-emit]

Let me see I have still my notes somewhere... I had to analyse the way it is generated to be able to decide how to implement the interceptor...

[Dhairya-Sangoi]

That would help a lot in understanding what happens under the hood. I was initially trying to look for some patterns by decompiling different methods having slightly different bodies.But it seems the compiler does its optimizations, so I was unable to identify any logical pattern.

[Dhairya-Sangoi]

Hey, any updates on this bug?

[Dhairya-Sangoi]

This is the observation based on different types of async functions:

Outside try-catch-finally	Try	Catch	Finally	Failure
3	0	0	0	0
2	1	0	0	1
2	0	1	0	1
2	0	0	1	0
1	2	0	0	0
1	0	2	0	0
1	0	0	2	0
1	1	1	0	0
1	1	0	1	1
1	0	1	1	1
0	3	0	0	0
0	0	3	0	0
0	0	0	3	0
0	2	1	0	0
0	2	0	1	0
0	1	2	0	1
0	1	0	2	1
0	1	1	1	0
0	0	2	1	0
0	0	1	2	1

The numbers in the cell indicate the number of await calls in that scope. Value of 1 in failure column indicates that the function failed at runtime.

[Dhairya-Sangoi]

I tried to distribute 3 async calls in different scopes. I didn't experiment it with 4 or more async calls. But my assumption is, if it fails for a particular type of distribution, then adding an extra async call in any scope would also fail. Though, I am not sure if the distributions which are passing could fail when additional async calls are added to scopes.

[reflection-emit]

I'll look into this asap… Thanks for the analysis. That helps me a lot.

[Dhairya-Sangoi]

Thanks!

[Dhairya-Sangoi]

Any luck with this?

[reflection-emit]

Yap... Fix is coming... I just have to write unit test to insure that it never comes up again.

---

Firma: Capgemini Deutschland GmbH Aufsichtsratsvorsitzender: Antonio Schnieder • Geschäftsführer: Dr. Michael Schulte (Sprecher) • Jost Förster • Dr. Peter Lempp • Dr. Volkmar Varnhagen

Amtsgericht Berlin-Charlottenburg, HRB 98814 This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.

[Dhairya-Sangoi]

Cool! Thanks for the update 👍

[srinivasupadhya]

any update on when this will be available? really looking forward for this fix. thanks :)

[reflection-emit]

Working on it right now... I am hoping to finish this today

[Dhairya-Sangoi]

Thanks for the fix!