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

What are the non thread-safe parts of EF? #5962

Closed
gdoron opened this issue Jul 3, 2016 · 10 comments
Closed

What are the non thread-safe parts of EF? #5962

gdoron opened this issue Jul 3, 2016 · 10 comments
Milestone

Comments

@gdoron
Copy link

gdoron commented Jul 3, 2016

Hi,

I just got hit by

A second operation started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe.

Are there APIs which are thread safe (like querying, adding instances to DbSet<T>) and some that are not or are they all non thread-safe?

@ajcvickers
Copy link
Member

@gdoron Work on the assumption that none of the code is thread safe. All of the main APIs, like DbContext, DbSet, etc. are not thread safe. There are some thread safe parts, which are usually singleton or similar shared services, but almost all of that is internal.

@gdoron
Copy link
Author

gdoron commented Jul 5, 2016

@ajcvickers thanks for the response.
Meaning even querying isn't thread safe? e.g. use the same DbContext instace for Querying is not thread safe?

If so, basically it means we can't share the same instance across methods and layers since they many times run in parallel.
Method X can't and shouldn't know if the same instance is being used somewhere else in a different thread.
So the DI shouldn't inject the same DbContext to multiple methods but a different one, or even better inject a service provider that generates DbContext, since a single method can run multiple DB queries in parallel.

Am I right?

@ajcvickers
Copy link
Member

@gdoron Correct. You should use short-lived context instances. A typical pattern in a web app is context per request. However, it is easy to instead use AddDbContext to register a transient service. If you need to create multiple instances you can easily do that too--exactly how depends on how you are using EF. In the simplest case, just new up instances of the context as necessary.

@ajcvickers ajcvickers added this to the Discussions milestone Jul 5, 2016
@gdoron
Copy link
Author

gdoron commented Jul 5, 2016

@ajcvickers
So as I see it, the default lifetime of AddDbContext really shouldn't be Scope but Transient as making it Scope can lead to race conditions.

And every class that is using EF should get a fresh DbContext instace.
And in case it runs multiple queries in parallel it should get IServiceProvider and resolve the dependency inside the method.
Correct?

Do you have any benchmarks of the cost of creating DbContext in Core?
I recall reading @rowanmiller comment somewhere that the creation of DbContext was optimized in Core, but he didn't provide numbers.

@ajcvickers
Copy link
Member

@gdoron I am not aware of a situation in a normal ASP.NET web app where you would have multiple concurrent threads executing for the same request. Beyond that, if the app does its own parallelization at any level then it is up to the app to take appropriate measures to make it safe.

We have benchmarks for context creation but I don't know if the numbers are public yet.

@gdoron
Copy link
Author

gdoron commented Jul 5, 2016

@ajcvickers I'll give you real examples we have in our application.
A vendor can upload his product with its images to our marketplace, each product can have n images.
We upload the images in parallel to Azure Blob Storage, and insert the URsL we are getting to the products table (and FileUploads table).

Also, when a user search on our marketplace, there are several categories, and we want to show him a little bit of all of them.
So we query the DB in parallel and await Task.When(queryATask, queryBTask, queryCTask)

I can give you many more examples in our application where we need to run multiple queries in a single request in parallel.

I think it's very common in web applications (if built right and optimized to utilize parallel programming).

@gdoron
Copy link
Author

gdoron commented Jul 5, 2016

@ajcvickers Regarding the numbers being private, since the product itself is public, I find very little sense in keeping the numbers of its benchmark private...
(Maybe I misunderstood what you meant)

@ajcvickers
Copy link
Member

@gdoron In cases where the app is introducing parallelization like that it would be up to the app to ensure appropriate handling of context instances and connections. I think we have all the pieces needed to do this, but it is not going to change the simple one context per request that we setup in the project templates and show in samples.

With regard to the numbers, it is not up to me whether they are made public. I believe the intention is to do so, but @rowanmiller would know more.

@gdoron
Copy link
Author

gdoron commented Jul 14, 2016

@rowanmiller can you share the benchmarks with the community? 🙏
Thanks.

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

For future visitors:

Currently it seems there's an issue with Identity and the transient DbContext.
Caution is advised.
aspnet/Identity#914

@ajcvickers I'm closing this issue, you obviously can reopen it if you want.

@gdoron gdoron closed this as completed Jul 27, 2016
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
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

2 participants