Cannot handle cyclic dependencies (StackOverflowExpeption)

[shmutalov]

Hello!

I just started writing Nancy.Bootstrapper wrapper over Grace IoC and collided with error. My lib: https://github.com/shmutalov/Unofficial.Nancy.Bootstrappers.Grace

All tests passed except one (if run them one by one), which causes a StackOverflowException:

• Should_resolve_module_with_dependency_on_RouteCacheFactory

Module that cannot be loaded: https://github.com/shmutalov/Unofficial.Nancy.Bootstrappers.Grace/blob/master/Unofficial.Nancy.Bootstrappers.Grace.Tests/Fakes/FakeNancyModuleWithRouteCacheProviderDependency.cs

NancyFx doc part about IRouteCacheProvider:

It's not safe for a module to take a dependency on the cache (cyclic dependency)

Best regards, Sherzod

[ipjohnson]

Hi @shmutalov

I'll try and take a look at this over the weekend. I've had Nancy running with Grace in the past so I don't envision this being a major change (fingers crossed).

[ipjohnson]

OK so I took a look this weekend and the issue revolves around the fact that SingletonLifestyle eagerly creates singletons for performance purposes. I added a deferred singleton lifestyle option for this case.

All that said Nancy's built in container is going to be much faster than child containers for Grace. It might be interesting to offer a second option that's based on lifetime scopes instead of child scopes. The downside of lifetime scopes is you lose the ability to export types in the per request registration on the other hand it should be at least1000x faster than Grace using child scopes.

Currently Nancy's built in container is very good at doing per request registration and I don't think there is a way to make Grace anywhere near as fast at per request registration. On the other hand I think lifetime scopes would be an interesting option if you don't need per request type registration as it should be much faster than Nancy.

Let me know if you are interested and I can put together an example of what I was thinking.

[shmutalov]

Let me know if you are interested and I can put together an example of what I was thinking.

Interested.

I looked to all foreign bootstrapper implementations for Nancy and saw that one or two of them implements per-request type registration. Maybe Grace implementation not need that too, idk

[ipjohnson]

Sorry I meant to respond to this earlier.

I was thinking you could do a wrapper like this and then the bootstrapper would look something like this.

As I'm not well versed with Nancy it's quite possible it's not exactly right but I think it's a decent start.

[ipjohnson]

@Shmutalov did my suggestion make sense?

[shmutalov]

Cyclic dependency test passed!

[ipjohnson]

Very cool. Let me know if the other two broken tests are an issue you want me to look at or related to the fact that per request registration is not supported.

[shmutalov]

IDK, is it per request registration issue or not. But test GetAllModule_Returns_As_MultiInstance fails with null check - container didn't located a module.

https://github.com/shmutalov/Unofficial.Nancy.Bootstrappers.Grace/blob/develop/Unofficial.Nancy.Bootstrappers.Grace.Tests/GraceNancyBootstrapperFixture.cs#L44

[ipjohnson]

I can take a look this evening

[ipjohnson]

@shmutalov so I took a look and it seems to be related to per request registration. You mentioned that a couple of the containers had implementation that didn't support per request registration. Can you point me in the direction of one or two of these implementations and I can investigate them.

[shmutalov]

1. Ninject https://github.com/NancyFx/Nancy.Bootstrappers.Ninject/blob/master/src/Nancy.Bootstrappers.Ninject/NinjectNancyBootstrapper.cs#L134

2. Unity https://github.com/NancyFx/Nancy.Bootstrappers.Unity/blob/master/src/Nancy.Bootstrappers.Unity/UnityNancyBootstrapper.cs#L151

3. Autofac https://github.com/NancyFx/Nancy.Bootstrappers.Autofac/blob/master/src/Nancy.Bootstrappers.Autofac/AutofacNancyBootstrapper.cs#L131

[ipjohnson]

@shmutalov I apologize I am interested in this topic I've just been swamped with family responsibilities so I haven't had much time to do open source work.

I took a look at those and I don't think it's doing exactly what you think. While the bootstrappers don't support the per request lifestyle I think they still support doing registration on every request which is roughly equivalent to per request lifestyle (I'm assuming they figured you'd do the latter if you want per request?).

In the coming days I'll see if I can find some time to post a question on the Nancy repository to see what their recommendation is for supporting a container that doesn't do per request registration. I don't think this is the first time someone has wanted to create a bootstrapper that does registration only at startup.

[ipjohnson]

@shmutalov I got some time last night to look at your tests some more. The two tests that are failing are most definitely failing because of

            container.Configure(registry =>
            {
                registry.Export<Foo>().As<IFoo>();
                registry.Export<Dependency>().As<IDependency>();
            });

The solution I'm purposing doesn't support doing registration in the ConfigureRequestContainer method.

So I think the bootstrapper is working correctly. That said I think it's probably better to inherit from the regular bootstrapper instead of the per request bootstrapper.

[ipjohnson]

I've released the defered singleton lifestyle as part of 6.4.1. I'm marking this as a question now as it's a question now for Nancy integration

[scovel]

Is DeferredSingletonLifestyle in the recent Beta? I have a need for it and it seems like it's not there.

[ipjohnson]

Hi scovel

It is but it’s not the default Singleton you have to add it by hand ‘Using(new DeferredSingletonLifestyle())’

I’ll send you an example this evening

[scovel]

Turns out Singleton worked just fine. I was worried it would create the instance immediately, but it doesn't.

[ipjohnson]

I’m going to close this out. If there is anything more we can reopen it