Merged PR for .NET 8 support #1
Merged
+2,484
−114
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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)
Threadpool.UnsafeQueueUserWorkItem.
this improve resolution of singleton object that might not need a root scope, the windsor container should be enough to resolve singletons
scopes from Threadpool.UnsafeQueueUserWorkItem
Major fix was to support keyed registration scenario
The previous handling of root scope is wrong, the code set root scope in an AsyncLocal variable when WindsorServiceProviderFactoryBase was first creatd. The problem arise with kestrel or orleans in .NET 8, they use a Thread Pool that runs code outside AsyncLocal so it will break resolution. It was not possible to reproduce locally, but it was reproduced with production code. A repro for the bug still missing. Also we can support using a Global root scope only if we use only ONE CONTAINER in the .NET core DI, because basic structure does not allow to find the container that is resolving scoped component thus we cannot determin the right root context. Still work to do to support multiple container.
Needed to modify basic Castle.Winsor library to support the ability from IHandler interface to get the current kernel associated with the handler. This is needed to find the correct root scope associated with that kernel instance.
The bug happened when you register a service with one NON keyed component then again with KEYED components. The adapter incorrectly checked only the first returned service for KEYED and returns null. Identified after update to Orleans 8.1.0
If multiple concrete classes are registered in castle, when you resolve castle resolves the first one. With Microsoft DI is the oposite, you want the last registered. The resolution is now fixed to honor IsDefault() because previous code registered every component with IsDefault() if it is registered from the adapter.
We added a new concept, an extendede property that allows the code to understand if the dependency was registered through the adapter (ServiceCollection) or directly through the Container. This allows us to change the resolution rule in case of multiple services registered with the same name.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merged PR 668 #668, created .net 8 versions of dlls and created local packages for internal consumption