feat: update to Autofac 5.1.0

[alsami]

• Update Autofac to version 5.1.0
• Update Castle.Core to version 4.4.0
• Change target-frameworks to net461;netstandard2.0;netstandard2.1 to match the once used for Autofac as well
• Update test dependencies and add different frameworks for the test projects

[alsami]

Some test for decorators are failing with Autofac version 5. Looking into it now.

Definitely no breaking changes because of immutable container though.

[alsami]

So the tests are all failing for the very same reason:

This is a DynamicProxy2 error: Target type for the proxy implements Castle.DynamicProxy.IProxyTargetAccessor which is a DynamicProxy infrastructure interface and you should never implement it yourself. Are you trying to proxy an existing proxy?

Something in the process of decorating must have changed with version 5. that is not compatible.

It happens in the class RegistrationExtensions where the ActivatingHandlers are added to the registration-builder.

[alsami]

Further observations:

This has definitely something to do with the way we decorate services now. Basically this library adds the following as an activation-handler:

EnsureInterfaceInterceptionApplies(e.Component);

var proxiedInterfaces = e.Instance
    .GetType()
    .GetInterfaces()
    .Where(ProxyUtil.IsAccessible)
    .ToArray();

if (!proxiedInterfaces.Any())
{
    return;
}

var theInterface = proxiedInterfaces.First();
var interfaces = proxiedInterfaces.Skip(1).ToArray();

var interceptors = GetInterceptorServices(e.Component, e.Instance.GetType())
    .Select(s => e.Context.ResolveService(s))
    .Cast<IInterceptor>()
    .ToArray();

e.Instance = options == null
    ? ProxyGenerator.CreateInterfaceProxyWithTarget(theInterface, interfaces, e.Instance, interceptors)
    : ProxyGenerator.CreateInterfaceProxyWithTarget(theInterface, interfaces, e.Instance, options, interceptors);

With version <= 4.9.4 the handler is called ones.

With version = 5.0.0 the handler is called twice which causes to proxy the proxy and therefor the error.

I haven't grasped all of the details by now but I am wondering if the changes from this PR would be of any help.

In the class InstanceLookup with version 5.0.0 it would access this piece of code twice

public object Execute()
{
    if (_executed)
        throw new InvalidOperationException(string.Format(CultureInfo.CurrentCulture, ComponentActivationResources.ActivationAlreadyExecuted, this.ComponentRegistration));

    _executed = true;

    object? decoratorTarget = null;
    object instance = ComponentRegistration.Sharing == InstanceSharing.None
        ? Activate(Parameters, out decoratorTarget)
        : _activationScope.GetOrCreateAndShare(ComponentRegistration.Id, () => Activate(Parameters, out decoratorTarget));

    var handler = InstanceLookupEnding;
    handler?.Invoke(this, new InstanceLookupEndingEventArgs(this, NewInstanceActivated));

    StartStartableComponent(decoratorTarget);

    return instance;
}

1. Immediately during the resolve process in the test, then the ActivatingHandler is being called
2. When the ActivatingHandler kicks in and executes the following

var interceptors = GetInterceptorServices(e.Component, e.Instance.GetType())
    .Select(s => e.Context.ResolveService(s))
    .Cast<IInterceptor>()
    .ToArray();

Furthermore this line

if (_newInstance != decoratorTarget)
   ComponentRegistration.RaiseActivating(this, resolveParameters, ref _newInstance);

In Activate of InstanceLookup causes the activator to be called twice.

A quick fix (dirty hack better to say) for this specific case is the following:

if (proxiedInterfaces.Any(@interface => @interface == typeof(IProxyTargetAccessor)))
{
    return;
}

If this is the case we know we already have decorated the service with a proxy. I checked in that hack for now. @tillig @alexmg any thoughts?

[alsami]

Hey guys, sorry for pushing that issue since users of Service-Fabric are waiting.

We need to figure out how to handle this. There is definitely a problem with the activator being called twice, apparently introduced by this PR.

The hack is fine and maybe that's the way to go now until we find out a possible solution, how to work around the activator being called twice.

What do you think @tillig @alexmg @alistairjevans ?

[alistairjevans]

The changes in autofac/Autofac#1072 do seem to resolve the problem (in the sense that the Activating callback isn't called twice anymore). Updating the DynamicProxy to latest 5.1-develop stops the duplicate invoke and tests pass.

Unless Autofac 5.1 is likely to ship very soon, my vote would be to release this integration with your workaround in place, then release again without the workaround once 5.1 ships.

I'd also suggest we add a new test in Autofac to specifically verify the Activation handler isn't called more than once if you have a decorator.

[alsami]

The changes in autofac/Autofac#1072 do seem to resolve the problem (in the sense that the Activating callback isn't called twice anymore). Updating the DynamicProxy to latest 5.1-develop stops the duplicate invoke and tests pass.

Hah, I knew it. I wonder if that could be released as a fix version instead?

Anyway, I am fine with that option as well so we can get things done and put it on the list of todos.

[alistairjevans]

My instinct is that the double invoke of Activating for decorated instances is a relatively major defect because of the impact it might have on people that use it (but I'm happy if it's not for some reason).

I'd say it's up to @tillig or @alexmg as to whether the severity warrants a more urgent 5.1 release. That choice will tell us what to do with this PR.

[alistairjevans]

It may be worth raising a new issue in the Autofac repo for the double-activation problem if there isn't one?

[tillig]

I think if we can get #1079 / #1080 resolved then issuing a 5.0.1 or whatever would be fine. I think @alexmg was looking at those.

[alsami]

It may be worth raising a new issue in the Autofac repo for the double-activation problem if there isn't one?

Would be a good so people who actually end up having this problem (meaning they are using decorators and also activation call backs) will find that it has already been solved but is not yet released.

[tillig]

Already got an issue associated with the PR. No need for another.

[alexmg]

I have released 5.1.0 to NuGet that has fixes for autofac/Autofac#1077 and autofac/Autofac#1073.

[alexmg]

You should be able to move forward now /cc @alistairjevans @alsami

[alexmg]

You should be able to remove the quick fix/hack now too.

[alsami]

You should be able to remove the quick fix/hack now too.

Awesome. Works now. Ready to review!

[tillig]

Looks great - thanks for keeping at this!