pamidur / aspect-injector

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

In a Advice method with Kind.Before and Target.Constructor, accessing mixed-in properties injected into a class containing an explicit constructor causes NullReferenceError. #221

Closed amguilmet closed 1 year ago

amguilmet commented 1 year ago

Environment (please complete the following information):

Describe the bug When trying to use a Scope.PerInstance Aspect to mix in an interface property, any attempt to access that property in an Advice method will throw a NullReferenceException when the Advice method uses both Kind.Before and Target.Constructor and the class upon which the aspect is applied has an explicit constructor. There are a few possible workarounds I've found so far, but they all have drawbacks in my case. You could: -omit the explicit constructor, but that's not feasible in classes that use DI with constructor injection. -use Kind.After, but that's not desirable in some cases eg. when trying use aspects to track object lifetimes. -use Scope.Global, but that makes mixin properties shared across all class instances.

To Reproduce

using AspectInjector.Broker;
namespace MixinTest;

internal class Program
{
    static void Main(string[] args)
    {
        using var instance = new Trackable();
        using var instance2 = new Trackable();
    }
}

[TrackLifetime]
public class Trackable : IDisposable
{
    public Trackable()
    {
        Console.WriteLine("Explicit Constructor");
    }

    public void Dispose()
    {
    }
}

public interface ITrackLifetime
{
    Guid Id { get; set; }
}

[Aspect(Scope.PerInstance)]
[Injection(typeof(TrackLifetime))]
[Mixin(typeof(ITrackLifetime))]
public class TrackLifetime : Attribute, ITrackLifetime
{
    public Guid Id { get; set; }

    [Advice(Kind.Before, Targets = Target.Constructor)]
    public void TrackableConstructor(
        [Argument(Source.Instance)] object instance)
    {
        Id = Guid.NewGuid(); //Exception occurs here
        Console.WriteLine($"Constructing {instance.GetType()} : {Id}");
    }

    [Advice(Kind.After, Targets = Target.Method)]
    public void TrackableDisposal(
        [Argument(Source.Name)] string name,
        [Argument(Source.Instance)] object instance)
    {
        if (name == nameof(IDisposable.Dispose))
            Console.WriteLine($"Disposed {instance.GetType()} : {Id}");

    }
}
pamidur commented 1 year ago

Thank you for the report, I will take a look asap.

On Fri, 14 Jul 2023, 06:27 amguilmet, @.***> wrote:

Environment (please complete the following information):

  • OS: windows
  • Framework: net 7
  • Type of application: console
  • Version of AspectInjector 2.8.2

Describe the bug When trying to use a Scope.PerInstance Aspect to mix in an interface property, any attempt to access that property in an Advice method will throw a NullReferenceException when the Advice method uses both Kind.Before and Target.Constructor and the class upon which the aspect is applied has an explicit constructor. There are a few possible workarounds I've found so far, but they all have drawbacks in my case. You could: -omit the explicit constructor, but that's not feasible in classes that use DI with constructor injection. -use Kind.After, but that's not desirable in some cases eg. when trying use aspects to track object lifetimes. -use Scope.Global, but that makes mixin properties shared across all class instances.

To Reproduce

using AspectInjector.Broker; namespace MixinTest;

internal class Program { static void Main(string[] args) { using var instance = new Trackable(); using var instance2 = new Trackable(); } }

[TrackLifetime] public class Trackable : IDisposable { public Trackable() { Console.WriteLine("Explicit Constructor"); }

public void Dispose()
{
}

}

public interface ITrackLifetime { Guid Id { get; set; } }

[Aspect(Scope.PerInstance)] [Injection(typeof(TrackLifetime))] [Mixin(typeof(ITrackLifetime))] public class TrackLifetime : Attribute, ITrackLifetime { public Guid Id { get; set; }

[Advice(Kind.Before, Targets = Target.Constructor)]
public void TrackableConstructor(
    [Argument(Source.Instance)] object instance)
{
    Id = Guid.NewGuid();
    Console.WriteLine($"Constructing {instance.GetType()} : {Id}");
}

[Advice(Kind.After, Targets = Target.Method)]
public void TrackableDisposal(
    [Argument(Source.Name)] string name,
    [Argument(Source.Instance)] object instance)
{
    if (name == nameof(IDisposable.Dispose))
        Console.WriteLine($"Disposed {instance.GetType()} : {Id}");

}

}

— Reply to this email directly, view it on GitHub https://github.com/pamidur/aspect-injector/issues/221, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7HZUHSJDIL2HLQXSUWQGTXQA4XVANCNFSM6AAAAAA2JKLBUM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

pamidur commented 1 year ago

for some reason aspect calls are attemted even before the aspects are initialized

image
pamidur commented 1 year ago

please let me know if 2.8.3-pre1 fixes it for you!

amguilmet commented 1 year ago

Works wonderfully!