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

Allow pooling DbContext with singleton services #27752

Closed
stevendarby opened this issue Apr 4, 2022 · 8 comments · Fixed by #30739
Closed

Allow pooling DbContext with singleton services #27752

stevendarby opened this issue Apr 4, 2022 · 8 comments · Fixed by #30739
Labels
area-dbcontext closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Milestone

Comments

@stevendarby
Copy link
Contributor

When using DbContext pooling, the DbContext can't have a constructor which takes anything but DbContextOptions. I would like to be able to inject singleton services - can this be allowed?

The example of how to use pooling in a multi-tenanted application is useful, however in my app we have a tenant ID accessor which is a singleton service that acts similarly to HttpContextAccessor. As such I was hoping to be able to inject this into my DbContext, even with pooling enabled. It's much nicer to be able to declare the dependencies in the constructor, and because it's a singleton service I don't think it matters that the DbContext instance is pooled?

If DbContext pooling is going to be pushed more aggressively, it might be nice to review ways to make it easier to use - such as this.

@ajcvickers
Copy link
Member

@stevendarby I think the solution for this is to inject a IDbContextFactory that uses pooling, which can itself take singleton services as dependencies and pass them into the created context instances. Nevertheless, I'll add this to the backlog as something we might support directly.

@stevendarby
Copy link
Contributor Author

stevendarby commented Apr 16, 2023

@ajcvickers thanks for the reply, just coming back this now and looking into your suggestion (I parked the idea of pooling at the time).

Am I right in thinking you're suggesting what is done here for the WeatherForeCastScopedFactory, where the ITenant is injected and set on the context after creation?

https://learn.microsoft.com/en-us/ef/core/performance/advanced-performance-topics?tabs=with-di%2Cexpression-api-with-constant#managing-state-in-pooled-contexts

If so, that is indeed an option right now but, just to clarify, what I am hoping for is a way to get a singleton dependency injected into the DbContext constructor and not need to set properties post-construction. If you're suggesting something else that does achieve that, I'd appreciate you expanding on that a bit please.

As it stands I think this is probably most easily achievable if EF PooledDbContextFactory did something like store the root service provider and use the Activator.CreateInstance method which takes the service provider and the DbContextOptions as an additional parameter, not sure about the performance of that though. It could be a fallback approach if there isn't the expected constructor. Or maybe an entirely different approach would be better!

P.S. I don't think there is really technical reason to have this other than it's a 'purer' way to do DI... e.g. be nice if I could continue to have readonly fields for those dependencies, it's a bit easier for someone looking at the DbContext to know what it depends on etc. etc.

@ajcvickers
Copy link
Member

@stevendarby There isn't a way to do this currently because injecting scoped services seemed like too much of a pit of failure, and, at the time we decided to just block all services rather than attempting to determine which services were safe. However, I'll bring this up again with the team.

@ajcvickers ajcvickers removed this from the Backlog milestone Apr 17, 2023
stevendarby pushed a commit to stevendarby/efcore that referenced this issue Apr 18, 2023
@stevendarby
Copy link
Contributor Author

stevendarby commented Apr 18, 2023

@ajcvickers See this fork PR for my idea for how it could work: stevendarby#1

edit: just discovered an assumption I had (from mostly working in ASP.NET Core in Development mode)- that scoped services can never be resolved from the root provider, but that's only true if validateScopes is true when building it.

However, I still think it's ok. The real pit of failure is never validating scopes (not even during development), and this would just be one more place where that pit of failure could manifest.

@ajcvickers
Copy link
Member

@stevendarby The pit of failure I am referring to is that the scoped service is resolved from the current scope--usually the current request scope--the first time the context is used, but then that same service is used for all subsequent uses of the context even though the underlying scope has changed. So, from a practical perspective, it looks like you are using a service scoped to the request, and when you run a simple test that is indeed the case, but then in most cases the service was actually created for a different request.

@stevendarby
Copy link
Contributor Author

stevendarby commented Apr 19, 2023

@ajcvickers I understand the problem using a scoped service in a pooled DbContext.

DbContextPool is a singleton service and so the service provider injected into it in my solution is the root service provider. The root service provider cannot create scoped services if it's configured to validate scopes, which is typically the case during Development time.

Therefore the code I shared, which creates the DbContext using whatever services can be resolved from the root service provider, plus the DbContextOptions, would typically throw at development time if the DbContext had scoped service dependencies.

At first I thought this was a fool proof way to allow singleton dependencies and block scoped dependencies, as I didn't realise that scope validation was a configurable feature. But in my post above I tried to reason that, if someone never validates scopes at any stage (e.g. at development time) and therefore ends up with a pooled DbContext that has scoped dependencies, the error would be in them never validating scopes and not EF Core for not blocking this. Or in other words, scoped services in singletons is a problem that could occur anywhere in the app if the developer doesn't validates scopes, and this would just one of many. We shift responsibility to the service provider and proper use of it.

@ajcvickers
Copy link
Member

Thanks, makes sense.

@ajcvickers
Copy link
Member

@stevendarby Looks good, @stevendarby. Happy to take a PR for this.

stevendarby pushed a commit to stevendarby/efcore that referenced this issue Apr 20, 2023
- Allow root provider DI for pooled DbContexts
@ajcvickers ajcvickers added this to the 8.0.0-preview4 milestone Apr 21, 2023
@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution labels Apr 21, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview4, 8.0.0 Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dbcontext closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants