Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected Torn Lifestyle warning when mixing Singletons and Collection #769

Closed
danielcrabtree opened this issue Nov 10, 2019 · 6 comments
Closed
Labels
Milestone

Comments

@danielcrabtree
Copy link

@danielcrabtree danielcrabtree commented Nov 10, 2019

Describe the bug

When types have been registered as singleton and those same types are registered into a collection, adding a type that depends on the collection results in a torn lifestyle when iterating the collection in the constructor. (This would appear to be related to changes for #554)

Expected behavior

When the collection is resolved, the singletons are returned as members of the collection, and it is thus OK to iterate them in the constructor of another singleton - same lifestyle.

Actual behavior

container.Verify() gives Torn Lifestyle errors:

-[Torn Lifestyle] The registration for I maps to the same implementation and lifestyle as the registration for A does. They both map to A (Singleton). This will cause each registration to resolve to a different instance: each registration will have its own instance.
-[Torn Lifestyle] The registration for A maps to the same implementation and lifestyle as the registration for I does. They both map to A (Singleton). This will cause each registration to resolve to a different instance: each registration will have its own instance.
-[Torn Lifestyle] The registration for I maps to the same implementation and lifestyle as the registration for B does. They both map to B (Singleton). This will cause each registration to resolve to a different instance: each registration will have its own instance.
-[Torn Lifestyle] The registration for B maps to the same implementation and lifestyle as the registration for I does. They both map to B (Singleton). This will cause each registration to resolve to a different instance: each registration will have its own instance.

To Reproduce

using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using SimpleInjector;

public interface I { }
public class A : I { }
public class B : I { }
public class U
{
    private I[] items;

    public U(IEnumerable<I> items)
    {
        this.items = items.ToArray();
    }
}

class Program
{
    static async Task Main(string[] args)
    {
        Container container = new Container();

        container.RegisterSingleton<U>();

        var types = container.GetTypesToRegister(
            typeof(I),
            typeof(I).Assembly);

        foreach(var type in types)
            container.RegisterSingleton(type, type);

        container.Collection.Register<I>(types);

        container.Verify();
    }
}

Additional context

This was working with SimpleInjector 4.0, I'm trying to upgrade to latest 4.7.

@danielcrabtree

This comment has been minimized.

Copy link
Author

@danielcrabtree danielcrabtree commented Nov 10, 2019

Manually registering the types as singletons using a shared registration resolves the warning.

var registrations = types
    .Select(t => (t, Lifestyle.Singleton.CreateRegistration(t, container)))
    .ToList();

foreach(var (type, registration) in registrations)
    container.AddRegistration(type, registration);

container.Collection.Register(typeof(I), registrations.Select(t => t.Item2));

I previously assumed collection registration caused the registered types to be resolved recursively, but it appears that registering a collection creates separate registrations.

Has the behavior of collection registration changed? Is there anyway to register a collection so it resolves the collection items rather than creates new registrations for them?

@danielcrabtree

This comment has been minimized.

Copy link
Author

@danielcrabtree danielcrabtree commented Nov 10, 2019

The collections documentation shows this example:

container.Register<MailLogger>(Lifestyle.Singleton);
container.RegisterInstance<ILogger>(new FileLogger());

container.Collection.Register<ILogger>(
    typeof(MailLogger),
    typeof(SqlLogger),
    typeof(ILogger));

So it appears that my assumption was correct and therefore registering the types as singletons and then registering them in a collection should work.

Therefore, either the torn lifestyle warning is wrong or Collection.Register is not detecting the existing singleton registrations and is re-registering them. I'm leaning towards Collection.Register issues since the manual workaround does not trigger torn lifestyle.

@dotnetjunkie dotnetjunkie added bug and removed question labels Nov 10, 2019
@dotnetjunkie dotnetjunkie added this to the v4.8 milestone Nov 10, 2019
@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

@dotnetjunkie dotnetjunkie commented Nov 10, 2019

Hi @danielcrabtree,

Thanks to your very detailed, high-quality, bug report I've been able to reproduce and track down this problem. This is indeed a bug. It was introduced in v4.0 with the implementation of #553. In the meantime, there are two ways for you to work around this issue:

  • Remove the call to .ToArray() from the constructor of U. Iterating injected collections is not adviced, but instead the storage of the original stream (e.g. IEnumerable<T>) is adviced. This is exactly what was implemented in #553.
  • Instead of using Collection.Register, use Collection.Append while applying a lifestyle. The bug only seems to appear when collection types are forwarded by using both Container.Collection.Register and Container.Register.

Here's an example of the second option:

var types = container.GetTypesToRegister(typeof(I), typeof(I).Assembly);

foreach(var type in types)
{
    container.Collection.Append<T>(type);
}
@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

@dotnetjunkie dotnetjunkie commented Nov 10, 2019

bug-769 branch created.

dotnetjunkie added a commit that referenced this issue Nov 10, 2019
@dotnetjunkie dotnetjunkie modified the milestones: v4.8, v4.8.1 Nov 17, 2019
@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

@dotnetjunkie dotnetjunkie commented Nov 30, 2019

This bug has been fixed in v4.8.1.

@danielcrabtree

This comment has been minimized.

Copy link
Author

@danielcrabtree danielcrabtree commented Dec 8, 2019

Confirmed, the bug is fixed in v4.8.1. Thank-you for resolving this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.