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

StackOverflowException when using Microsoft.Extensions.DependencyInjection #62239

Closed
huysentruitw opened this issue Dec 1, 2021 · 21 comments
Closed

Comments

@huysentruitw
Copy link

huysentruitw commented Dec 1, 2021

Update: It also occurs in a ASP.NET Core API project (see below)

Describe the bug

I am currently working on a ASP.NET MVC5 project that targets .NET472. A lot of business logic needs to be shared with a ASP.NET Core 6 project, so we're using multi-target to do that. Because of the same reason, we have switched our dependency injection from Unity to Microsoft.Extensions.DependencyInjection, so both application (old MVC5 + new ASP.NET Core6 API) can use the same way of registering services.

However, now we have switched to Microsoft.Extensions.DependencyInjection v5.0.2 (can't use 6.0.0 because of an incompatible 3rd party package), we're hitting the same StackOverflowException as described here dotnet/aspnetcore#2737. Every other request gives us that exception.

The workaround described here no longer works, because the options object has been changed, but the idea behind it still works: switch from dynamic provider engine to the runtime provider engine.

A similar workaround for MS DI version 5.0.2 looks like this:

var serviceProviderType = typeof(ServiceProvider);
var serviceProviderEngineInterfaceType = serviceProviderType.Assembly.GetType("Microsoft.Extensions.DependencyInjection.ServiceLookup.IServiceProviderEngine");
var serviceProviderConstructor = serviceProviderType.GetConstructor(
    BindingFlags.NonPublic | BindingFlags.Instance,
    null,
    new[]
    {
        typeof(IServiceCollection),
        serviceProviderEngineInterfaceType,
        typeof(ServiceProviderOptions)
    },
    null);

var runtimeEngineType = serviceProviderType.Assembly.GetType("Microsoft.Extensions.DependencyInjection.ServiceLookup.RuntimeServiceProviderEngine");
var runtimeEngineConstructor = runtimeEngineType.GetConstructor(new[] { typeof(IServiceCollection) });

var engine = runtimeEngineConstructor.Invoke(new[] { services });
var serviceProvider = serviceProviderConstructor.Invoke(new[] { services, engine, new ServiceProviderOptions() });

return (IServiceProvider)serviceProvider;

instead of simply calling:

services.BuildServiceWorker();

I have seen that the code for MS DI 6 has changed considerably, so the workaround would also need to be fixed in the future (unless we get everything on .NET 6 in the meantime).

However, my question is: why is the dynamic engine causing this issue and would it be possible to add a setting to the ServiceProviderOptions that lets us select the engine mode?

Further technical details

  • ASP.NET version: 5
  • ASP.NET Core version: 6
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and its version: VS2019 / VS2022 / Rider
@huysentruitw
Copy link
Author

Oops, wrong repository. Can someone move this issue to dotnet/runtime please 😬?

@TanayParikh TanayParikh transferred this issue from dotnet/aspnetcore Dec 1, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-Extensions-DependencyInjection untriaged New issue has not been triaged by the area owner labels Dec 1, 2021
@ghost
Copy link

ghost commented Dec 1, 2021

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

Describe the bug

I am currently working on a ASP.NET MVC5 project that targets .NET472. A lot of business logic needs to be shared with a ASP.NET Core 6 project, so we're using multi-target to do that. Because of the same reason, we have switched our dependency injection from Unity to Microsoft.Extensions.DependencyInjection, so both application (old MVC5 + new ASP.NET Core6 API) can use the same way of registering services.

However, now we have switched to Microsoft.Extensions.DependencyInjection v5.0.2 (can't use 6.0.0 because of an incompatible 3rd party package), we're hitting the same StackOverflowException as described here dotnet/aspnetcore#2737. Every other request gives us that exception.

The workaround described here no longer works, because the options object has been changed, but the idea behind it still works: switch from dynamic provider engine to the runtime provider engine.

A similar workaround for MS DI version 5.0.2 looks like this:

var serviceProviderType = typeof(ServiceProvider);
var serviceProviderEngineInterfaceType = typeof(ServiceProvider).Assembly.GetType("Microsoft.Extensions.DependencyInjection.ServiceLookup.IServiceProviderEngine");
var serviceProviderConstructor = serviceProviderType.GetConstructor(
    BindingFlags.NonPublic | BindingFlags.Instance,
    null,
    new[]
    {
        typeof(IServiceCollection),
        serviceProviderEngineInterfaceType,
        typeof(ServiceProviderOptions)
    },
    null);

var runtimeEngineType = typeof(ServiceProvider).Assembly.GetType("Microsoft.Extensions.DependencyInjection.ServiceLookup.RuntimeServiceProviderEngine");
var runtimeEngineConstructor = runtimeEngineType.GetConstructor(new[] { typeof(IServiceCollection) });

var engine = runtimeEngineConstructor.Invoke(new[] { services });
var serviceProvider = serviceProviderConstructor.Invoke(new[] { services, engine, new ServiceProviderOptions() });

return (IServiceProvider)serviceProvider;

instead of simply calling:

services.BuildServiceWorker();

I have seen that the code for MS DI 6 has changed considerably, so the workaround would also need to be fixed in the future (unless we get everything on .NET 6 in the meantime).

However, my question is: why is the dynamic engine causing this issue and would it be possible to add a setting to the ServiceProviderOptions that lets us select the engine mode?

Further technical details

  • ASP.NET version: 5
  • ASP.NET Core version: 6
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and its version: VS2019 / VS2022 / Rider
Author: huysentruitw
Assignees: -
Labels:

untriaged, area-Extensions-DependencyInjection

Milestone: -

@huysentruitw
Copy link
Author

huysentruitw commented Dec 1, 2021

I have created a minimal repro https://github.com/huysentruitw/MsDiStackOverflowDemo where the StackOverflowException occurs when clicking around through the menu.

Starting from a scaffolded .NET472 MVC project, I have done these changes: huysentruitw/MsDiStackOverflowDemo@136c63c

I'm generating a dependency tree using the Services\TestServices.tt T4 template. I had to play a bit with the service count at line 10 in the T4 template in order to provoke the StackOverflowException.

The service count is currently set to 30. A lower count like 20 doesn't provoke it. Using higher count like 100 even causes the page to never show up (probably another issue 🙂). It also only occurs when multiple services are using the same service. When I just create a chain like: S1 -> S2 -> S3 -> S4, I can go up to 5000 without any problem.

Good luck ❤️

@huysentruitw
Copy link
Author

huysentruitw commented Dec 1, 2021

The bad news is that it also affects an ASP.NET Core API project. I have added a sample project here: huysentruitw/MsDiStackOverflowDemo@7665b8f

Just refresh the weatherforcast endpoint a few times to get the StackOverflowException.

@huysentruitw huysentruitw changed the title StackOverflowException when using MS DI in .NET472 based MVC5 application StackOverflowException when using Microsoft.Extensions.DependencyInjection Dec 1, 2021
@maryamariyan maryamariyan added this to the 7.0.0 milestone Dec 1, 2021
@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Dec 1, 2021
@eerhardt
Copy link
Member

eerhardt commented Dec 6, 2021

Using the above code, I was able to pare this down to the following repro:

using Microsoft.Extensions.DependencyInjection;

static class Program
{
    static void Main()
    {
        var sp = new ServiceCollection()
            .AddServices()
            .BuildServiceProvider();

        var serviceType = typeof(Service1);
        sp.GetRequiredService(serviceType);
        sp.GetRequiredService(serviceType);

        // give enough time for the background Ref.Emit to run
        Thread.Sleep(5000);

        sp.GetRequiredService(serviceType);
    }

    static IServiceCollection AddServices(this IServiceCollection services)
    {
        foreach (Type serviceType in typeof(Program).Assembly.GetTypes()
            .Where(type => !type.IsAbstract && typeof(IService).IsAssignableFrom(type)))
        {
            services.AddTransient(serviceType);
        }

        return services;
    }
}

public interface IService { }
public class Service1 : IService
{
    public Service1(Service2 serviceA, Service3 serviceB) { }
}
public class Service2 : IService
{
    public Service2(Service3 serviceA, Service4 serviceB) { }
}
public class Service3 : IService
{
    public Service3(Service4 serviceA, Service5 serviceB) { }
}
public class Service4 : IService
{
    public Service4(Service5 serviceA, Service6 serviceB) { }
}
public class Service5 : IService
{
    public Service5(Service6 serviceA, Service7 serviceB) { }
}
public class Service6 : IService
{
    public Service6(Service7 serviceA, Service8 serviceB) { }
}
public class Service7 : IService
{
    public Service7(Service8 serviceA, Service9 serviceB) { }
}
public class Service8 : IService
{
    public Service8(Service9 serviceA, Service10 serviceB) { }
}
public class Service9 : IService
{
    public Service9(Service10 serviceA, Service11 serviceB) { }
}
public class Service10 : IService
{
    public Service10(Service11 serviceA, Service12 serviceB) { }
}
public class Service11 : IService
{
    public Service11(Service12 serviceA, Service13 serviceB) { }
}
public class Service12 : IService
{
    public Service12(Service13 serviceA, Service14 serviceB) { }
}
public class Service13 : IService
{
    public Service13(Service14 serviceA, Service15 serviceB) { }
}
public class Service14 : IService
{
    public Service14(Service15 serviceA, Service16 serviceB) { }
}
public class Service15 : IService
{
    public Service15(Service16 serviceA, Service17 serviceB) { }
}
public class Service16 : IService
{
    public Service16(Service17 serviceA, Service18 serviceB) { }
}
public class Service17 : IService
{
    public Service17(Service18 serviceA, Service19 serviceB) { }
}
public class Service18 : IService
{
    public Service18(Service19 serviceA, Service20 serviceB) { }
}
public class Service19 : IService
{
    public Service19(Service20 serviceA, Service21 serviceB) { }
}
public class Service20 : IService
{
    public Service20(Service21 serviceA, Service22 serviceB) { }
}
public class Service21 : IService
{
    public Service21(Service22 serviceA, Service23 serviceB) { }
}
public class Service22 : IService
{
    public Service22(Service23 serviceA, Service24 serviceB) { }
}
public class Service23 : IService
{
    public Service23(Service24 serviceA, Service25 serviceB) { }
}
public class Service24 : IService
{
    public Service24(Service25 serviceA, Service26 serviceB) { }
}
public class Service25 : IService
{
    public Service25(Service26 serviceA, Service27 serviceB) { }
}
public class Service26 : IService
{
    public Service26(Service27 serviceA, Service28 serviceB) { }
}
public class Service27 : IService
{
    public Service27(Service28 serviceA, Service29 serviceB) { }
}
public class Service28 : IService { }
public class Service29 : IService { }

The issue is that all these services are "transient", meaning we will create a new instance each time one is asked. Looking at the above dependency structure, you can see that when I create a Service26, I need a Service27 and a Service28. Then when I create a Service27, it also needs a Service28, so another new one is created there. Now imagine that same issue played out 30 times, the problem is O(n!) - it is basically a fibonacci sequence of instances being created.

To prove this out, here is the dump of IL code generated for the above program, but changing serviceType to typeof(Service22).

	IL_0000: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_0005: newobj instance void [ConsoleApp41]Service29::.ctor()
	IL_000a: newobj instance void [ConsoleApp41]Service27::.ctor(class [ConsoleApp41]Service28, class [ConsoleApp41]Service29)
	IL_000f: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_0014: newobj instance void [ConsoleApp41]Service26::.ctor(class [ConsoleApp41]Service27, class [ConsoleApp41]Service28)
	IL_0019: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_001e: newobj instance void [ConsoleApp41]Service29::.ctor()
	IL_0023: newobj instance void [ConsoleApp41]Service27::.ctor(class [ConsoleApp41]Service28, class [ConsoleApp41]Service29)
	IL_0028: newobj instance void [ConsoleApp41]Service25::.ctor(class [ConsoleApp41]Service26, class [ConsoleApp41]Service27)
	IL_002d: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_0032: newobj instance void [ConsoleApp41]Service29::.ctor()
	IL_0037: newobj instance void [ConsoleApp41]Service27::.ctor(class [ConsoleApp41]Service28, class [ConsoleApp41]Service29)
	IL_003c: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_0041: newobj instance void [ConsoleApp41]Service26::.ctor(class [ConsoleApp41]Service27, class [ConsoleApp41]Service28)
	IL_0046: newobj instance void [ConsoleApp41]Service24::.ctor(class [ConsoleApp41]Service25, class [ConsoleApp41]Service26)
	IL_004b: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_0050: newobj instance void [ConsoleApp41]Service29::.ctor()
	IL_0055: newobj instance void [ConsoleApp41]Service27::.ctor(class [ConsoleApp41]Service28, class [ConsoleApp41]Service29)
	IL_005a: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_005f: newobj instance void [ConsoleApp41]Service26::.ctor(class [ConsoleApp41]Service27, class [ConsoleApp41]Service28)
	IL_0064: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_0069: newobj instance void [ConsoleApp41]Service29::.ctor()
	IL_006e: newobj instance void [ConsoleApp41]Service27::.ctor(class [ConsoleApp41]Service28, class [ConsoleApp41]Service29)
	IL_0073: newobj instance void [ConsoleApp41]Service25::.ctor(class [ConsoleApp41]Service26, class [ConsoleApp41]Service27)
	IL_0078: newobj instance void [ConsoleApp41]Service23::.ctor(class [ConsoleApp41]Service24, class [ConsoleApp41]Service25)
	IL_007d: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_0082: newobj instance void [ConsoleApp41]Service29::.ctor()
	IL_0087: newobj instance void [ConsoleApp41]Service27::.ctor(class [ConsoleApp41]Service28, class [ConsoleApp41]Service29)
	IL_008c: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_0091: newobj instance void [ConsoleApp41]Service26::.ctor(class [ConsoleApp41]Service27, class [ConsoleApp41]Service28)
	IL_0096: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_009b: newobj instance void [ConsoleApp41]Service29::.ctor()
	IL_00a0: newobj instance void [ConsoleApp41]Service27::.ctor(class [ConsoleApp41]Service28, class [ConsoleApp41]Service29)
	IL_00a5: newobj instance void [ConsoleApp41]Service25::.ctor(class [ConsoleApp41]Service26, class [ConsoleApp41]Service27)
	IL_00aa: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_00af: newobj instance void [ConsoleApp41]Service29::.ctor()
	IL_00b4: newobj instance void [ConsoleApp41]Service27::.ctor(class [ConsoleApp41]Service28, class [ConsoleApp41]Service29)
	IL_00b9: newobj instance void [ConsoleApp41]Service28::.ctor()
	IL_00be: newobj instance void [ConsoleApp41]Service26::.ctor(class [ConsoleApp41]Service27, class [ConsoleApp41]Service28)
	IL_00c3: newobj instance void [ConsoleApp41]Service24::.ctor(class [ConsoleApp41]Service25, class [ConsoleApp41]Service26)
	IL_00c8: newobj instance void [ConsoleApp41]Service22::.ctor(class [ConsoleApp41]Service23, class [ConsoleApp41]Service24)
	IL_00cd: ret

You can see that the following instances are being created:

Type Instance count
Service22 1
Service23 1
Service24 2
Service25 3
Service26 5
Service27 8
Service28 13

@huysentruitw - do you really have this scenario in a production app? Would it be better if you were to use Singleton or Scoped services instead of Transient?

If we needed to make this scenario work, we would probably need to store the created instances in local variables instead of using the IL stack to hold onto the created instances.

@eerhardt eerhardt modified the milestones: 7.0.0, Future Dec 6, 2021
@eerhardt
Copy link
Member

eerhardt commented Dec 6, 2021

Moving to Future, as I don't see this scenario being very common. We can pull it back into 7.0 if we get enough evidence that this must be fixed now.

@huysentruitw
Copy link
Author

huysentruitw commented Dec 6, 2021

Thanks for looking into it!

As stated in my original post, we're migrating from Unity to MS DI. In Unity, the default is transient, so we thought registering everything as transient would give us the least surprises after this migration. Also note, that we're using MS DI in a legacy .NET Framework 4.7.2 ASP.NET MVC5 setup, so the repro code is only a guess of what could be going wrong in the original application. But making everything scoped is something we definitely are going to try.

@eerhardt
Copy link
Member

eerhardt commented Dec 6, 2021

can't use 6.0.0 because of an incompatible 3rd party package

It would be great if you could try to use Microsoft.Extensions.DependencyInjection 6.0.0 on your original application. Can you say which 3rd party package doesn't support 6.0, but does support 5.0?

@huysentruitw
Copy link
Author

It would be great if you could try to use Microsoft.Extensions.DependencyInjection 6.0.0 on your original application. Can you say which 3rd party package doesn't support 6.0, but does support 5.0?

It's CsvHelper 27.1.1 which requires Microsoft.Bcl.AsyncInterfaces v5.0.0. But I see that in the latest version of CsvHelper, this dependency is relaxed, so might try again in upcoming days.

@eerhardt
Copy link
Member

eerhardt commented Dec 6, 2021

But I see that in the latest version of CsvHelper, this dependency is relaxed

Nice. Yes, in general having = dependencies in your NuGet packages are bad for this exact reason. I'm glad CsvHelper appears to have removed them - JoshClose/CsvHelper@78f84d1.

@Hjaltesorgenfrei
Copy link

We are having the same issue while migrating from Unity to MS DI. I have tested against Microsoft.Extensions.DependencyInjection 6.0.0 and the stack overflow still happens.

@eerhardt
Copy link
Member

eerhardt commented Mar 3, 2022

@Hjaltesorgenfrei - is it possible to send a minimal repro of your scenario? That would help us understand and prioritize fixing this issue.

@Hjaltesorgenfrei
Copy link

Our scenario is essentially the one you mentioned above in your minimal example, except that we use ServiceCollection.GetServices instead.
So I don't know if an minimal repro would be helpful as it would be very similar to yours.
When I lower the amount of services in your example to 26 the memory usage is also lower, 1.4GB instead of 6.2GB, but I still get the stack overflow.
This is the same memory usage our application has when it stack overflows,
If we run our application with some of it's services removed it only reaches 1.1GB before the resolve finishes and does not overflow the stack.
So we reckon we are just over the limit for how large the dependency graph can be.

@Hjaltesorgenfrei
Copy link

Also it appears that the memory usage is only high upon the resolve call which Emit is done in.
Any subsequent calls to GetRequiredService after the call where memory usage spikes are much faster and have no real impact upon memory usage.
Is it therefore possible that the culprit is GenerateMethodBody in ILEmitResolverBuilder and not creating the actual objects?

@eerhardt
Copy link
Member

eerhardt commented Mar 4, 2022

Our scenario is essentially the one you mentioned above in your minimal example

You have this much recursion in your dependency graph? Note that my minimal example is basically a recursive fibannoci growth of the number of services being created. Are you sure the services need to be Transient?

@Hjaltesorgenfrei
Copy link

Yep, we have quite a lot of recursion in our graph. It is of course possible to refactor our application to avoid using Transient all the way down.
But it would be nice if the "default" worked, that being that everything is Transient unless there is a need for it to be scoped or a singleton. Or if one could choose which ServiceProviderEngine was used, so that we could avoid the problem in the ILEmitter.

@defcon84
Copy link

@ericstj This issue was planned for .NET 7, but not fixed. Now it isn't even planned for .NET 8, any idea why not?

@eerhardt
Copy link
Member

@defcon84 - would it be possible to show a real-world example of where you hit this bug?

@defcon84
Copy link

defcon84 commented Jul 12, 2023

@eerhardt

With the scope of our enterprise project we have an estimate of 450 registered dependencies. These are separated primarily in 3 layers; Controller, Service and Dao.
The services are almost exclusively registered as Scoped. About 400 Scoped, 50 Singleton and 10 Transient. Many of these Scoped services depend on request scoped mechanisms (identity, database transactions, etc).

Because of the large scale of our project and our dependencies branching wide rather than deep we're hitting this limit even without many Transient services. For example, there are instances where a Service layer dependency can inject upwards to a few dozen Dao dependencies.
Splitting up services further increases the dependency tree making us run into this problem faster. Combining code from different services into one to reduce dependencies would be troublesome to maintain.

It might be possible to further reduce the dependency tree by creating singleton services and accessing the scope within functions, though when looking for similar questions this also doesn't seem like a great idea.

We've switched to DryIoc after trying a couple of IoC containers with at least one running into a similar IL stack issue and some other ones not being able to handle the size of the project altogether. Due to concerns regarding memory usage we wanted to revisit the official Microsoft Dependency Injection.

I can't share any of the code but I could spend some time working out the hierarchy into a demo project. That said I very much doubt the result would be much different from your repro in this comment.

@eerhardt
Copy link
Member

That said I very much doubt the result would be much different from #62239 (comment).

A big difference between my repro above and what you are describing is that my repro above used all Transient services - and there was a recursive Fibonacci sequence for resolving them - which exploded the graph quite quickly. In my opinion, this repro isn't a "real world" scenario, which is why I'm looking for a real world scenario that runs into the same problem it does.

In your description, you only have 10 Transient services, which should be well under the stack limit to create, even if you tried to resolve them all at once.

Would it be possible to post a stack trace of the exception? Even better would be to get a crash dump of the exception. That would give us more information on exactly is going wrong with your application.

@defcon84
Copy link

@eerhardt We are unable to reproduce it in .NET 7, so this can be closed.

@buyaa-n buyaa-n closed this as not planned Won't fix, can't repro, duplicate, stale Sep 13, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants