-
Notifications
You must be signed in to change notification settings - Fork 458
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
Use Root scope when current is null (fixes #646) #670
base: master
Are you sure you want to change the base?
Use Root scope when current is null (fixes #646) #670
Conversation
I still ask to check for our pr #668 that will fix this and also add support for .NET 8 keyed services. If it is possible could you check code of pr #668 to verify if this fixes null scope in your situation? I'm asking because the support for .NET 8 branch is opened from a long time and I'd like it to be closed if possible (we are in production with that version using a private package source and I'd really like to move again on supported nuget packages) |
@alkampfergit i had a quick look, but need to read a lot about semantics (especially keyed / not keyed). I would like to implement it using named instances in Castle so we don't have to introduce new concepts. |
Remember that if you are moving to dependency injection version 8.0.0 without keyed service support the adapter does not works . The problem presents when any library starts using keyed service and new resolution semantic, so you got weird error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add at least one unit test for this change.
/// <summary>Current scope for the thread. Initial scope will be set when calling BeginRootScope from a ExtensionContainerRootScope instance.</summary> | ||
/// <exception cref="InvalidOperationException">Thrown when there is no scope available.</exception> | ||
internal static ExtensionContainerScopeBase Current | ||
{ | ||
get => current.Value ?? throw new InvalidOperationException("No scope available"); | ||
set => current.Value = value; | ||
} | ||
|
||
internal static ExtensionContainerScopeBase TryCurrent => current.Value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TryCurrent
is a strange name, it's not a function. Any reason not to just name it Current
and just document that is can return null?
Fixes #646 by using Root scope when current scope in
ExtensionContainerScopeCache.current
isnull
. This can happen whenWindsorScopedServiceProvider
is called from a new thread (or rather different async context).In this situation
ForcedScope
uses the original root scope. It also restores it asExtensionContainerScopeCache.current
on dispose.The only case that I haven't figured out is when resolving directly via
IWindsorContainer
(and not throughIServiceProvider
) on a new thread.ExtensionContainerScopeAccessor.GetScope
still throws "No scope available"