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

Iterating collection with scoped dependencies in singleton consumer accidentally succeeds inside Verify() #554

Open
dotnetjunkie opened this Issue May 11, 2018 · 1 comment

Comments

1 participant
@dotnetjunkie
Collaborator

dotnetjunkie commented May 11, 2018

Consider the following code:

public class Consumer
{
    private readonly ILogger[] loggers;
    public Consumer(IEnumerable<ILogger> loggers)
    {
        this.loggers = loggers.ToArray();
    }
}

Using this class, the following code will fail:

var container = new Container();
container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();

container.Register<NullLogger>(Lifestyle.Scoped);
container.RegisterCollection<ILogger>(new[] { typeof(NullLogger) });
container.Register<Consumer>(Lifestyle.Singleton);

container.GetInstance<Consumer>();

With the following exception:

The NullLogger is registered as 'Async Scoped' lifestyle, but the instance is requested outside the context of an active (Async Scoped) scope.

However, when a Verify() call is added, it accidentally succeeds:

var container = new Container();
container.Options.DefaultScopedLifestyle = new AsyncScopedLifestyle();

container.Register<NullLogger>(Lifestyle.Scoped);
container.RegisterCollection<ILogger>(new[] { typeof(NullLogger) });
container.Register<Consumer>(Lifestyle.Singleton);

container.Verify(); // added

container.GetInstance<Consumer>(); // now succeeds

What happens is the following:

  • When running inside Verify(), Simple Injector implicitly creates a Scope to allow Scoped instances to be resolved.
  • Since Verify() created the Singleton Consumer, the collection is iterated inside a Scope. This means creation of Consumer succeeds.
  • Since Consumer is a Singleton, it will be cached and the following GetInstance<Consumer> call simply returns that cached instance.

This might actually be quite tricky to fix, because Verify() requires a Scope to function. This would mean that Scope should be suppressed during construction of Singleton instances. This however might be a breaking change.

@dotnetjunkie dotnetjunkie added this to the Backlog milestone May 11, 2018

@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Jun 13, 2018

Perhaps the problem will automatically be solved when #553 is implemented?

@dotnetjunkie dotnetjunkie modified the milestones: Backlog, v4.4 Jul 14, 2018

@dotnetjunkie dotnetjunkie modified the milestones: v4.4, v4.5 Sep 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment