Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async Method Weaving: OnEnter() is called multiple times #66

Closed
Dhairya-Sangoi opened this issue Aug 1, 2018 · 3 comments
Closed

Async Method Weaving: OnEnter() is called multiple times #66

Dhairya-Sangoi opened this issue Aug 1, 2018 · 3 comments

Comments

@Dhairya-Sangoi
Copy link

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:

	class TestingClass
	{
		[TestingAnnotation]
		public async Task AsyncTestMethod(string inp1, int inp2)
		{
			Console.WriteLine("inside async test method");
			await Task.Delay(100);
			Console.WriteLine("inside async test method");
			await Task.Delay(100);
			Console.WriteLine("inside async test method");
			await Task.Delay(100);
		}
	}

Decompiled weaved code:

[CompilerGenerated]
    private sealed class \u003CAsyncTestMethod\u003Ed__0 : IAsyncStateMachine
    {
      public int \u003C\u003E1__state;
      public AsyncTaskMethodBuilder \u003C\u003Et__builder;
      public string inp1;
      public int inp2;
      public TestingClass \u003C\u003E4__this;
      private TaskAwaiter \u003C\u003Eu__1;
      public IMethodInterceptor \u003CAsyncTestMethod\u003E_2yaukd2v55stk;

      public \u003CAsyncTestMethod\u003Ed__0()
      {
        base.\u002Ector();
      }

      void IAsyncStateMachine.MoveNext()
      {
        int num1 = this.\u003C\u003E1__state;
        try
        {
          try
          {
            object[] values = new object[2]
            {
              (object) this.inp1,
              (object) this.inp2
            };
            if (num1 != 0)
            {
              // ISSUE: method reference
              // ISSUE: type reference
              this.\u003CAsyncTestMethod\u003E_2yaukd2v55stk.OnEnter(typeof (TestingClass), (object) this.\u003C\u003E4__this, MethodBase.GetMethodFromHandle(__methodref (TestingClass.AsyncTestMethod), __typeref (TestingClass)), values);
            }
            TaskAwaiter awaiter1;
            int num2;
            TaskAwaiter awaiter2;
            TaskAwaiter awaiter3;
            switch (num1)
            {
              case 0:
                awaiter1 = this.\u003C\u003Eu__1;
                this.\u003C\u003Eu__1 = new TaskAwaiter();
                this.\u003C\u003E1__state = num1 = -1;
                break;
              case 1:
                awaiter2 = this.\u003C\u003Eu__1;
                this.\u003C\u003Eu__1 = new TaskAwaiter();
                this.\u003C\u003E1__state = num1 = -1;
                goto label_10;
              case 2:
                awaiter3 = this.\u003C\u003Eu__1;
                this.\u003C\u003Eu__1 = new TaskAwaiter();
                this.\u003C\u003E1__state = num1 = -1;
                goto label_13;
              default:
                Console.WriteLine("inside async test method");
                awaiter1 = Task.Delay(100).GetAwaiter();
                if (!awaiter1.IsCompleted)
                {
                  this.\u003C\u003E1__state = num2 = 0;
                  this.\u003C\u003Eu__1 = awaiter1;
                  TestingClass.\u003CAsyncTestMethod\u003Ed__0 stateMachine = this;
                  this.\u003C\u003Et__builder.AwaitUnsafeOnCompleted<TaskAwaiter, TestingClass.\u003CAsyncTestMethod\u003Ed__0>(ref awaiter1, ref stateMachine);
                  return;
                }
                break;
            }
            awaiter1.GetResult();
            Console.WriteLine("inside async test method");
            awaiter2 = Task.Delay(100).GetAwaiter();
            if (!awaiter2.IsCompleted)
            {
              this.\u003C\u003E1__state = num2 = 1;
              this.\u003C\u003Eu__1 = awaiter2;
              TestingClass.\u003CAsyncTestMethod\u003Ed__0 stateMachine = this;
              this.\u003C\u003Et__builder.AwaitUnsafeOnCompleted<TaskAwaiter, TestingClass.\u003CAsyncTestMethod\u003Ed__0>(ref awaiter2, ref stateMachine);
              return;
            }
label_10:
            awaiter2.GetResult();
            Console.WriteLine("inside async test method");
            awaiter3 = Task.Delay(100).GetAwaiter();
            if (!awaiter3.IsCompleted)
            {
              this.\u003C\u003E1__state = num2 = 2;
              this.\u003C\u003Eu__1 = awaiter3;
              TestingClass.\u003CAsyncTestMethod\u003Ed__0 stateMachine = this;
              this.\u003C\u003Et__builder.AwaitUnsafeOnCompleted<TaskAwaiter, TestingClass.\u003CAsyncTestMethod\u003Ed__0>(ref awaiter3, ref stateMachine);
              return;
            }
label_13:
            awaiter3.GetResult();
            if (num1 != 0)
              ;
          }
          finally
          {
            this.\u003CAsyncTestMethod\u003E_2yaukd2v55stk.OnExit();
          }
        }
        catch (Exception ex)
        {
          if (this.\u003CAsyncTestMethod\u003E_2yaukd2v55stk.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 call to OnEnter is based on the following condition:

if (num1 != 0)
            {
              // ISSUE: method reference
              // ISSUE: type reference
              this.\u003CAsyncTestMethod\u003E_2yaukd2v55stk.OnEnter(typeof (TestingClass), (object) this.\u003C\u003E4__this, MethodBase.GetMethodFromHandle(__methodref (TestingClass.AsyncTestMethod), __typeref (TestingClass)), values);
            }

I think, the condition should be this instead:

if (num1 == -1)
            {
              // ISSUE: method reference
              // ISSUE: type reference
              this.\u003CAsyncTestMethod\u003E_2yaukd2v55stk.OnEnter(typeof (TestingClass), (object) this.\u003C\u003E4__this, MethodBase.GetMethodFromHandle(__methodref (TestingClass.AsyncTestMethod), __typeref (TestingClass)), values);
            }

num1 is the current state of the state machine which is created for this method. A call to OnEnter should only happen if the current state is the initial state (-1).

reflection-emit added a commit that referenced this issue Aug 2, 2018
OnExit is also changed to be always executed after OnException. And also just 1 time.
@reflection-emit
Copy link
Collaborator

I uploaded a new package with the fix. Please confirm

@Dhairya-Sangoi
Copy link
Author

Thanks for the quick fix. Will confirm and let you know.

@Dhairya-Sangoi
Copy link
Author

Confirmed. Works as expected! Thanks for the quick fix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants