Circular component dependency detected with manual IEnumerable<T> registration

[felixwatts]

Here is a console application that causes a DependencyResolutionException. There is no actual problem with the dependency tree. The error message wrongly states that IEnumerable<IPlugin> depends on PluginsViewModel.

A first chance exception of type 'Autofac.Core.DependencyResolutionException' occurred in Autofac.dll

Additional information: Circular component dependency detected: AutofacBug.RootViewModel -> System.Collections.Generic.IEnumerable`1[[AutofacBug.IPlugin, AutofacBug, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]] -> AutofacBug.PluginsViewModel -> System.Collections.Generic.IEnumerable`1[[AutofacBug.IPlugin, AutofacBug, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].

using System.Collections.Generic;
using System.Linq;
using Autofac;
using Autofac.Core;

namespace AutofacBug
{
    class Program
    {
        static void Main(string[] args)
        {
            var containerBuilder = new ContainerBuilder();

            containerBuilder.RegisterType<RootViewModel>().AsSelf().SingleInstance();
            containerBuilder.RegisterType<PluginsViewModel>().AsSelf().SingleInstance();

            RegisterPlugins(containerBuilder);

            var container = containerBuilder.Build();

            // the exception on this line wrongly states that IEnumerable<IPlugin> depends on PluginsViewModel. It does not and can be resolved.
            var rootViewModel = container.Resolve<RootViewModel>();
        }

        private static void RegisterPlugins(ContainerBuilder containerBuilder)
        {
            containerBuilder.RegisterType(typeof(Plugin1)).Named<IPlugin>("Plugin1");
            containerBuilder.RegisterType(typeof(Plugin2)).Named<IPlugin>("Plugin2");
            containerBuilder.Register(SafeResolveAllPlugins).As<IEnumerable<IPlugin>>().SingleInstance();
        }

        private static IEnumerable<IPlugin> SafeResolveAllPlugins(IComponentContext core)
        {
            var sucessfullyResolvedPlugins =
                new[] {"Plugin1", "Plugin2"}
                .Select(name => SafeResolvePlugin(name, core))
                .Where(p => p != null)
                .ToArray();

            return sucessfullyResolvedPlugins;
        }

        private static IPlugin SafeResolvePlugin(string pluginName, IComponentContext core)
        {
            try
            {
                // The exception on this line is normal and expected because the plugin requires an unavailable component
                return core.ResolveNamed<IPlugin>(pluginName);
            }
            catch (DependencyResolutionException ex)
            {
                return null;
            }
        }
    }

    public interface IPlugin
    {
    }

    class Plugin1 : IPlugin
    {
    }

    class Plugin2 : IPlugin
    {
        public Plugin2(IUnavailableComponent unavailableComponent)
        {
        }
    }

    internal interface IUnavailableComponent
    {
    }

    class PluginsViewModel
    {
        public PluginsViewModel(IEnumerable<IPlugin> plugins)
        {

        }
    }

    class RootViewModel
    {
        public RootViewModel(IEnumerable<IPlugin> plugins, PluginsViewModel pluginsViewModel)
        {

        }
    }
}

[tillig]

Interesting. I'm guessing it's got something to do with both RootViewModel and PluginsViewModel needing IEnumerable<IPlugin> and the explicit registration of IEnumerable<IPlugin>.

RootViewModel
|    |
|    +------> IEnumerable<IPlugin> -----> Custom delegate singleton
|             ^
|             |
+-----> PluginsViewModel

We'll have to investigate how to address that. It's a bit of an edge case - most folks don't explicitly register the IEnumerable<T> relationship for a component type, let alone as a singleton.

[tillig]

This may or may not tie into the newly opened design-based issue #798.

[SimonCropp]

out of curiosity i smoke tested this in 4.7. the above code now results in a

'Autofac.Core.Activators.Reflection.DefaultConstructorFinder' on type 'Plugin2' can be invoked with the available services and parameters:
Cannot resolve parameter 'IUnavailableComponent unavailableComponent' of constructor 'Void .ctor(IUnavailableComponent)'.'

which i believe is the correct behaviour? and hence this can be closed?

[tillig]

Nice catch. Let me add a unit test in for this to make sure we don't regress and if it passes we can call it good.

[tillig]

Unfortunately it looks like this one still fails. I added a failing unit test here (marked skip) so we can easy go back and test to see if it's fixed, but... not quite fixed yet. Feel free to check out the unit test to see if I incorrectly specified it.

[tillig]

I did figure out that, as I suspected, it has to do with the explicit registration of IEnumerable<IPlugin>. If I modify the test to just register IPlugin implementations directly (and omit broken ones) then no circular dependency is detected. It's only when the explicit registration is introduced that the circular dependency check starts failing.

[tillig]

Not clear if it overlaps, but we do have #798 where we're looking at improving the circular dependency handling.

I think this has to do with the way the CollectionRegistrationSource handles generating the registrations for IEnumerable<T>. I walked through the code a few times from the failing test and it still wasn't clear exactly where the issue is.

It occurred to me that it might be better to solve the problem by creating a RegistrationSource rather than overriding IEnumerable<T> but it'd still be good to figure out why this is happening.

[alexmg]

I just ran the skipped unit test (ManualEnumerableRegistrationDoesNotCauseCircularDependency) and it is now passing. Any objections to this issue being closed considering the test is no longer failing?

[tillig]

I'm not sure what changed to make it pass, but if it's passing, it sounds like this is resolved. Maybe some of the internals work that's happened in the last 10-ish months?

[tillig]

We've uncommented the test as part of PR #962 to ensure there's no regression here. It's passing and, ideally, should continue to pass so... I'm closing this.