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

Refs: #646 Castle.Windsor.Extension.DepencencyInjection - No Scope Available #664

Conversation

AGiorgetti
Copy link

ExtensionContainerScopeCache.current is an AsyncLocal, AsyncLocal might not be captured in threads originating from Threadpool.UnsafeQueueUserWorkItem.

AspNetCore make heavy use of such threads to optimize performances.

I Added some tests that show supported and unsupported scenarios

I moved the null checks from the AsyncLocal Cache to the scope accessors and improved error messages a bit.

Added an experimental feature to use Castle Windsor standard Singleton lifecycle instead of the custom NetStatic (I believe singletons should always be resolved if you have a container instance whatever the scope is). Enabling it helps me solve singleton resolution issues when the same components are resolved from the IServiceProvider and/or WindsorContainer.

However having "null" for the current scope might be questionable, the Dispose in WindsorScopedServiceProvider rely on that information to correctly cleanup the things (looking at the dotnet DI code, the root scope should be strictly tied to the root service provider).
This behavior might still be good because disposing a ServiceProvider resolved/used in a background thread should not dispose the main container anyway.

I think that more changes should be done like:

  • do not dispose the container in WindsorScopedServiceProvider, but dispose it in the factory (only if it creates the container instance)
  • create the root scope in the factory CreateServiceProvider() function rather than in the constructor (it should be more closer to what Microsoft DI does).

There was a previous PR (that I closed) similar to this one with less tests and a fix that didn't worked in some scenario, I clsed that and replaced with this one. I think some more changes can be made and a lot of intermediate commits can be squased.

…removed null

check on scope cache (AsyncLocal can be null on Threads coming from
Threadpool.UnsafeQueueUserWorkItem, having no null check was also the
original behavior)
this improve resolution of singleton object that might not need a root
scope, the windsor container should be enough to resolve singletons
@AGiorgetti
Copy link
Author

I am closing this PR because #668 superseeds this one.
It also add KeyedServices support

@AGiorgetti AGiorgetti closed this Feb 15, 2024
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

Successfully merging this pull request may close these issues.

1 participant