pamidur / aspect-injector

AOP framework for .NET (c#, vb, etc)
Apache License 2.0
745 stars 112 forks source link

Is this a bug of execution's sequence? #163

Closed ziokuroi closed 3 years ago

ziokuroi commented 3 years ago

Some of example code:

public static void Main(string[] args) { RedisHelper.Initialization(new CSRedisClient("127.0.0.1:6379,ssl=false,poolsize=8")); Console.WriteLine("Test Start!"); Service.Test(); Console.ReadKey(); }

[After] [Around] public async Task Test() { Console.WriteLine("Test"); return await Test2(); }

    [Around]
    public async Task<string> Test2() {
        Console.WriteLine("Test2");
        return await RedisHelper.GetAsync<string>("test");
    }

[Aspect(Scope.Global)] [Injection(typeof(AfterAttribute))] [AttributeUsage(AttributeTargets.Method)] public class AfterAttribute : System.Attribute {

    [Advice(Kind.After)]
    public void After1([Argument(Source.Name)] string name) {
        Console.WriteLine(name + " After");
        Intercept();
    }

    private static async void Intercept() {
        await RedisHelper.SetAsync("test", "test");
    }

}

[Aspect(Scope.Global)] [Injection(typeof(AroundAttribute))] [AttributeUsage(AttributeTargets.Method)] public class AroundAttribute : System.Attribute{

    private static readonly MethodInfo AsyncHandler = typeof(AroundAttribute).GetMethod(nameof(AroundAttribute.InterceptAround), BindingFlags.Public | BindingFlags.Static);

    [Advice(Kind.Around)]
    public object Around(
        [Argument(Source.ReturnType)] Type returnType,
        [Argument(Source.Arguments)] object[] args,
        [Argument(Source.Target)] Func<object[],object> method,
        [Argument(Source.Name)] string name) {
        Console.WriteLine(name + " around in");
        if (returnType.BaseType == typeof(Task) && returnType.IsGenericType) {
            var resultType = returnType.IsConstructedGenericType ? returnType.GenericTypeArguments[0] : typeof(object);
            var result1 = AsyncHandler.MakeGenericMethod(resultType).Invoke(this, new object[] { method, args });
            Console.WriteLine(name + " around out");
            return result1;
        }
        Console.WriteLine(name + " around out");
        var result = method(args);
        return result;
    }

    public static async Task<TResult> InterceptAround<TResult>(Func<object[],object> method, object[] args) {
        return await (Task<TResult>) method(args);
    }

}

And I print every steps of interceptor to show execution sequence. I think the sequence will be like this:

Test Around in Test Test2 Around in Test2 Test2 Around out Test Around out Test After

But actually, the sequence is:

Test around in Test Test2 around in Test2 Test2 around out Test After Test around out

The [Test After] runs after [Test2 Around] Not [Test Around].

The full test project i have uploaded to https://github.com/ziokuroi/Test

Thanks everyone :)

pamidur commented 3 years ago

Hi @ziokuroi, thank you for interest in this project. The behaviour you see is by design. Before and After aspects work in the deepest possible level, as close as possible as original code. So Before will execute right before the original code, After will execute right after the original code. The Priority only applies within the kind of aspects. Priotiry for Around will be separate from priority for After.

If this behaviour is not what you're looking for I would suggest to use only Around aspects.

I hope it helps!

ziokuroi commented 3 years ago

@pamidur I wonder whether there is an option that can controll the Before and After aspects work in the deepest possible level or in the level where attributes on. If it doesn't exist, then this is my suggestion for a new version. 😄 But anyway, Thanks for your reply :)

pamidur commented 3 years ago

It is controlled like this https://github.com/pamidur/aspect-injector/blob/5b809ec584bb794f63b4c193638efef49b1de2c7/src/AspectInjector.Core.Advice/Weavers/AdviceInlineWeaver.cs#L16

Values are hardcoded, but should easy to introduce some logic. Pull requests are welcome!

ziokuroi commented 3 years ago

@pamidur I found this maybe a bug of when using aspect on async methods I add a test for non-async, and the sequence behavior is what i expect:

Test around in Test Test2 around in Test2 Test2 around out Test around out Test After

TestAsync around in TestAsync TestAsync2 around in TestAsync2 TestAsync2 around out TestAsync After TestAsync around out

As you see, the non-async test, After has been executed after Around. But the async test, After has been executed after the deepest method. I have update the Test Project at https://github.com/ziokuroi/Test Thank you :)

pamidur commented 3 years ago

After in Sync and Async methods behave differently. After in async methods is put at latest stage of async execution. In other words - try to await the Task ( task= (Task)target(args)) in your Around aspect and see if it makes any difference.

I agree, this behavior isn't particularly clear and easy to understand. It is like that historically and for a reason that After advice should work the same - for async and sync. What you're facing is the side effect. I'm looking to change that in next major release 3.x. No ETA but suggestions are welcome.

ziokuroi commented 3 years ago

@pamidur Okay, got it, thank you :)