Capgemini / Cauldron

C# Toolkit
MIT License
76 stars 18 forks source link

Async Method Weaving: Parameters-Array value is not set when passing the array to OnEnter() #65

Closed Dhairya-Sangoi closed 5 years ago

Dhairya-Sangoi commented 5 years ago

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)
        {
            await Task.Delay(1000);
        }

    }

Decompiled weaved code:

internal class TestingClass
  {
    [DebuggerStepThrough]
    [AsyncStateMachine(typeof (TestingClass.\u003CAsyncTestMethod\u003Ed__0))]
    public Task AsyncTestMethod(string inp1, int inp2)
    {
      IMethodInterceptor methodInterceptor = (IMethodInterceptor) new TestingAnnotation();
      TestingClass.\u003CAsyncTestMethod\u003Ed__0 stateMachine = new TestingClass.\u003CAsyncTestMethod\u003Ed__0();
      stateMachine.\u003C\u003E4__this = this;
      stateMachine.inp1 = inp1;
      stateMachine.inp2 = inp2;
      stateMachine.\u003C\u003Et__builder = AsyncTaskMethodBuilder.Create();
      stateMachine.\u003C\u003E1__state = -1;
      stateMachine.\u003CAsyncTestMethod\u003E_gtqmvl2i59zeb = methodInterceptor;
      stateMachine.\u003C\u003Et__builder.Start<TestingClass.\u003CAsyncTestMethod\u003Ed__0>(ref stateMachine);
      return stateMachine.\u003C\u003Et__builder.Task;
    }

    public TestingClass()
    {
      base.\u002Ector();
    }

    [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_gtqmvl2i59zeb;

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

      void IAsyncStateMachine.MoveNext()
      {
        int num1 = this.\u003C\u003E1__state;
        try
        {
          try
          {
            object[] values = new object[2];
            if (num1 != 0)
            {
              // ISSUE: method reference
              // ISSUE: type reference
              this.\u003CAsyncTestMethod\u003E_gtqmvl2i59zeb.OnEnter(typeof (TestingClass), (object) this.\u003C\u003E4__this, MethodBase.GetMethodFromHandle(__methodref (TestingClass.AsyncTestMethod), __typeref (TestingClass)), values);
            }
            TaskAwaiter awaiter;
            if (num1 != 0)
            {
              awaiter = Task.Delay(1000).GetAwaiter();
              if (!awaiter.IsCompleted)
              {
                int num2;
                this.\u003C\u003E1__state = num2 = 0;
                this.\u003C\u003Eu__1 = awaiter;
                TestingClass.\u003CAsyncTestMethod\u003Ed__0 stateMachine = this;
                this.\u003C\u003Et__builder.AwaitUnsafeOnCompleted<TaskAwaiter, TestingClass.\u003CAsyncTestMethod\u003Ed__0>(ref awaiter, ref stateMachine);
                return;
              }
            }
            else
            {
              awaiter = this.\u003C\u003Eu__1;
              this.\u003C\u003Eu__1 = new TaskAwaiter();
              this.\u003C\u003E1__state = num1 = -1;
            }
            awaiter.GetResult();
            if (num1 != 0)
              ;
          }
          finally
          {
            this.\u003CAsyncTestMethod\u003E_gtqmvl2i59zeb.OnExit();
          }
        }
        catch (Exception ex)
        {
          if (this.\u003CAsyncTestMethod\u003E_gtqmvl2i59zeb.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 object[] values in IAsyncStateMachine.MoveNext() is not getting initialized with the actual parameters that should be passed to the method.

This bug seems to be arising from Cauldron\Fody\Cauldron.Interception.Cecilator\Coders\Default\Coder.cs file.

I think this could fix the bug:

In GetParametersArray() method, changing:

foreach (var parameter in associatedMethod.methodReference.Parameters)
                    {
                        newBlock.Emit(OpCodes.Ldloc, variable.variable);
                        newBlock.Append(InstructionBlock.CreateCode(newBlock, null, counter++));
                        newBlock.Append(InstructionBlock.CreateCode(newBlock, BuilderTypes.Object, associatedMethod.AsyncMethodHelper.AsyncStateMachineType.GetField(parameter.Name)));
                        newBlock.Emit(OpCodes.Stelem_Ref);
                    }

to

foreach (var parameter in associatedMethod.AsyncMethodHelper.Method.methodReference.Parameters)
                    {
                        newBlock.Emit(OpCodes.Ldloc, variable.variable);
                        newBlock.Append(InstructionBlock.CreateCode(newBlock, null, counter++));
                        newBlock.Append(InstructionBlock.CreateCode(newBlock, BuilderTypes.Object, associatedMethod.AsyncMethodHelper.AsyncStateMachineType.GetField(parameter.Name)));
                        newBlock.Emit(OpCodes.Stelem_Ref);
                    }

This could be happening because, in case of async methods, the associatedMethod is an object of type AsyncStateMachineMoveNextMethod for which the associatedMethod.methodReference.Parameters.Count is 0.

reflection-emit commented 5 years ago

Hi I will look into this and fix this asap.


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.

reflection-emit commented 5 years ago

I uploaded a new package with the fix. Please confirm

Dhairya-Sangoi commented 5 years ago

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

Dhairya-Sangoi commented 5 years ago

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