search

Elfocrash / Cosmonaut

🌐 A supercharged Azure CosmosDB .NET SDK with ORM support
https://cosmonaut.readthedocs.io
MIT License
342 stars 44 forks source link

Cosmonaut deadlocking when constructed using IOC #77

Closed roberino closed 5 years ago

roberino commented 5 years ago

Hey there....nice library (mostly!)

It seems that because Cosmonaut is doing some synchronous initialisation of the collection and database, deadlocking can occur under certain conditions.

I think it might be good to at least make this behaviour configurable, since in many situations, it would be better to fail (e.g. there might be numerous indexing and partitioning requirements for a collection and I would rather know if I'd forgotten to provision it properly rather than the client library blindly moving ahead) but also, blocking asynchronous code within the constructor seems a little dodgy.

We can replicate this problem specifically by registering the CosmosStore as an open generic and then attempting to resolve a class which has two ICosmosStore parameters within the constructor but I haven't been able to work out exactly why this would behave like this. Also, I've only been able to replicate this within an ASP.NET core web application. It seems fine within a console app (although a little slow to initialise).

Example registration code:

services.AddSingleton(typeof(ICosmosStore<>), typeof(CosmosStore<>));

Reference to blocking async calls: https://github.com/Elfocrash/Cosmonaut/blob/d61919d59b3823a861548740134e0d6004d4ac54/src/Cosmonaut/CosmosStore.cs#L252

Elfocrash commented 5 years ago

Hello @roberino.

The reason why it's deadlocking is because, with your approach, the CosmosStore won't be initialised until it's first request from the app. In some weird way, which I haven't been able to replicate, this call will be coming from the UI context and will cause the deadlock. The reason why the console app won't fail is because there is no UI thread.

Initialising the CosmosStore before adding in the IoC would solve the problem. I still don't understand why this is only happening with the typeof registration and not the generic type one as well though. Technically they should both fail in the same way.

Anyway, I am happy to add a config value in the CosmosStoreSettings named ProvisionResourcesIfMissing which would only provision the database and collection if set to true. This would also have to be the default behaviour as it would be a breaking change doing otherwise.

I could also add an extra method called EnsureResourcesProvisionedAsync to manually called the provisioning methods if your disable auto provisioning.

How do those things sound?

Elfocrash commented 5 years ago

Addressed in 2.10.0.

You can now opt out of automatic infrastructure provisioning by setting ProvisionInfrastructureIfMissing in CosmosStoreSetting to false.

You can also provision on demand by calling EnsureInfrastructureProvisionedAsync from the CosmosStore.

Does this solve your problem?

roberino commented 5 years ago

Awesome work!

Thanks...will test out the new version Monday.

BTW...I realised that when registering the class using a factory method, the same issue occurred so I guess it's something to do with the synchronisation logic in the IOC but I haven't looked deeper into it.