ArxOne / MrAdvice

.NET aspect weaver (build task under NuGet package)
MIT License
311 stars 45 forks source link

Instance of advice attribute is created twice. #144

Closed AlexanderButs closed 3 years ago

AlexanderButs commented 5 years ago

Hi again :)

Another issue from my side.

I've updated my project to use MrAdvice from v.2.0.16 to v2.8.5 and noticed strange thing - the actual advice attribute instance is created twice. To check this behavior just put some output log into advice attribute ctor and You'll see this log twice. I declare advice this way:

[AttributeUsage(AttributeTargets.Method, AllowMultiple = true, Inherited = false)]
public class AdviceAsyncAttribute : Attribute, IMethodAsyncAdvice
{
    public AdviceAsyncAttribute()
    {
        Console.WriteLine("ctor AdviceAsyncAttribute");
    }

    public async Task Advise(MethodAsyncAdviceContext context)
    {
        Console.WriteLine("Before MethodAsync");
        await context.ProceedAsync();
        Console.WriteLine("After MethodAsync");
    }
}

This behavior is on Windows with .NET Framework.

picrap commented 5 years ago

Is it possible that you inject the same advice at different level, such as assembly+type, or type+method?

AlexanderButs commented 5 years ago

Nope. I created separated solution to test this behavior. Using this advice only for async method in one place. 2.0.16 advice instance is created only once. 2.8.5 advice instance is created twice.

picrap commented 5 years ago

There are unit tests in solution that ensure an advice is applied only once when it should be, so I'm pretty sure that in common case the system works as expected. Which means there is something in your sample out of common case, and I'd like to know what.

AlexanderButs commented 5 years ago

Sure, I'll provide my example a little bit later :)

AlexanderButs commented 5 years ago

Here is my test example. Hope it will run without any issues.

TestMrAdvice.zip

picrap commented 5 years ago

Confirmed! Thanks. I need to figure out why, but right, in your sample, the attribute is instantiated twice. This should not be a problem, since nothing should be stored in it (you can introduce fields if you want to store data in instances). However, I'll investigate on it.

AlexanderButs commented 5 years ago

Correct, method is intercepted correctly as it did before. I just rely on previous behavior in unit tests - I set some static variable in advice ctor and then verify some actions. That's how I found this bug - second instance just override my static variable and verification fails.

Delectus98 commented 4 years ago

Same issue for IMethodAdvice. I want to store an unique id inside a static Dictionnary for each of my own IMethodAdvice but then unique id is used multiples times because the IMethodAdvice instance is created twice.

picrap commented 4 years ago

Yes, there is no guarantee an attribute is created once (or twice). So if you want to store extra data related to instances, you can use introduced fields, they're designed for it. More details at https://github.com/ArxOne/MrAdvice/wiki/IntroducingFields