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

Application breaks after updating to v6.3.5+ (LightInject.Web.PerWebRequestScopeManager.GetOrAddScope()) #559

Open
vstadn opened this issue Oct 12, 2021 · 5 comments

Comments

@vstadn
Copy link

vstadn commented Oct 12, 2021

Hey, I have a .NET 472 mvc application running in production that uses LightInject and LightInject.Web packages. I have an internal package that is basically a wrapper around LightInject package.After updating to v6.3.5 or later, I started experiencing 2 different issues.

1 - My package did not pass Lifetime parameter for Override method. With the update, all of the registrations that were overriden started acting as transient - numerous instances of objects were created. I fixed this with passing Lifetime parameter into Override method (as part of the ServiceRegistration object that's returned from a func parameter).

2 - The application has numerous background services that are triggered on a timer. The application uses EnablePerWebRequestScope extension method to use PerWebRequestScopeManagerProvider.

Object reference not set to an instance of an object.\n\rObject reference not set to an instance of an object.",
    "StackTrace": "   at LightInject.Web.PerWebRequestScopeManager.GetOrAddScope() in C:\\projects\\lightinject-web\\build\\tmp\\Net46\\Binary\\LightInject.Web\\LightInject.Web.cs:line 148

I started getting these errors for all of the timer-driven services. After looking at the source code, I suspect the issue is that GetOrAddScope() function tries to access HttpContext.Current which will be null in this case. I have copied over the LightInject.Web.cs file and added a null check for HttpContext.Current. While this fixed my errors for background services, I started getting multiple instances of the services that have Override registrations.

Is this something you have encountered before?

@vstadn
Copy link
Author

vstadn commented Oct 12, 2021

To add, I have confirmed on my end that the issue of multiple instances is caused by Override method. Even though I am passing PerContainerLifetime in the registration func, the objects are created numerous times.

@sirphilliptubell
Copy link

I tried to repro this in a new project, but couldn't. And the project it was happening in was too large to trim down to find a minimal repro.

However I found another of our projects has the same issue in another way. This other project is net6.0, and uses .CreateServiceProvider() (in LightInject.Microsoft.DependencyInjection) to create the service provider and in an IHostedService I was able to find this scenario:

// 6.3.4 returns 20 instances
// 6.3.5 throws stack overflow exception
var instances = ServiceProvider.GetServices<IHandler>(); // the Microsoft.Extensions.DependencyInjection extension

// but....

// works in both 6.3.4 and 6.3.5
var types = new [] { type1, type2, ... type20 };  // each type is an instance of IHandler and is the actual type that would be returned above
foreach (var t in types) {
    ServiceProvider.GetService(t);  // works
}

I was able to identify commit e803e57 as problematic, while commit daaf6a0 is ok by building both commits and copying over the lightinject.dll to our application.

@sirphilliptubell
Copy link

I tried removing the implementations of IHandler one by one. With 13 types it works, with 14 types it throws

// throws with 14 types with 6.3.5
// takes ~10 seconds with 13 types with 6.3.5
// super fast with 6.3.4
var instances = ServiceProvider.GetServices<IHandler>(); 

var types = new [] { type1, type2, ... type13_or_14 };  // each type is a class implementing IHandler
foreach (var t in types) {
    // takes ~1 second per call (varies) with 13 types with 6.3.5
    // fast with 6.3.4
    ServiceProvider.GetService(t);  // works
}

I suspect we might have a problem somewhere with singleton types depending on transient or scoped types, but it will take some time to investigate whether this is true.

@seesharper
Copy link
Owner

@sirphilliptubell First of all, I really appreciate the time you have put into investigating this. If you could narrow this down into a working repro that would be great although I understand it might be difficult to do so.
A couple of things. The reason for the difference in performance for resolving the services is probably because in 6.3.4 we resolved the instance at "compile time" and put that instance into play as a constant. This means that any time spent in the constructor of the service would not be part of the GetInstance call.
You mentioned that another project uses LightInject.Microsoft.DependencyInjection. Does it work if you use the default provider?

@sirphilliptubell
Copy link

sirphilliptubell commented Aug 6, 2022

fyi, just realized I meant to reply to issue #561, not this one. Sorry to throw this thread off it's rails. I'll continue posting in the other thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants