From 08ad3dce59ad6ea5801e0d5291381667f03fa8b0 Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Fri, 4 Aug 2023 17:16:48 +0200 Subject: [PATCH 1/9] Refs: #646 Castle.Windsor.Extension.DepencencyInjection 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) --- .../ResolveFromThreadpoolUnsafe.cs | 56 +++++++++++++++++++ .../Scope/ExtensionContainerScopeCache.cs | 2 +- 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs diff --git a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs new file mode 100644 index 0000000000..754ea0381f --- /dev/null +++ b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs @@ -0,0 +1,56 @@ +using Castle.MicroKernel.Registration; +using Castle.Windsor.Extensions.DependencyInjection.Tests.Components; +using Microsoft.Extensions.DependencyInjection; +using System; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Castle.Windsor.Extensions.DependencyInjection.Tests { + public class ResolveFromThreadpoolUnsafe { + [Fact] + public async Task Can_Resolve_From_ServiceProvider() { + + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + /* + ThreadPool.UnsafeQueueUserWorkItem(state => { + // resolving using castle (without scopes) works + var actualUserService = container.Resolve(nameof(UserService)); + Assert.NotNull(actualUserService); + }, null); + */ + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => { + try { + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + } +} diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs index d89f9dd1f2..1e094869c6 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs @@ -24,7 +24,7 @@ internal static class ExtensionContainerScopeCache /// Thrown when there is no scope available. internal static ExtensionContainerScopeBase Current { - get => current.Value ?? throw new InvalidOperationException("No scope available"); + get => current.Value; // ?? throw new InvalidOperationException("No scope available"); set => current.Value = value; } } From a8347ea39d932bf48e036075ef7c2518871960f4 Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Fri, 4 Aug 2023 18:54:39 +0200 Subject: [PATCH 2/9] ExtensionContainerRootsScopeAccessor might return a null root scope in Threadpool.UnsafeQueueUserWorkItem. --- .../Scope/ExtensionContainerRootScopeAccessor.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs index 25139efec9..d6c34e3099 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs @@ -12,22 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -namespace Castle.Windsor.Extensions.DependencyInjection.Scope -{ +namespace Castle.Windsor.Extensions.DependencyInjection.Scope { using System; using Castle.MicroKernel.Context; using Castle.MicroKernel.Lifestyle.Scoped; - internal class ExtensionContainerRootScopeAccessor : IScopeAccessor - { - public ILifetimeScope GetScope(CreationContext context) - { + internal class ExtensionContainerRootScopeAccessor : IScopeAccessor { + public ILifetimeScope GetScope(CreationContext context) { + if (ExtensionContainerScopeCache.Current == null) { + // might be null in threads spawn from Threadpool.UnsafeQueueUserWorkItem + return null; + } return ExtensionContainerScopeCache.Current.RootScope ?? throw new InvalidOperationException("No root scope available"); } - public void Dispose() - { + public void Dispose() { } } } From ba03d1bd4ec5064660c1721dc338d1b063f9633e Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Mon, 7 Aug 2023 13:41:53 +0200 Subject: [PATCH 3/9] Added More failing test cases --- .../ResolveFromThreadpoolUnsafe.cs | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs index 754ea0381f..3aa8469fe1 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs @@ -1,4 +1,5 @@ using Castle.MicroKernel.Registration; +using Castle.Windsor.Extensions.DependencyInjection.Extensions; using Castle.Windsor.Extensions.DependencyInjection.Tests.Components; using Microsoft.Extensions.DependencyInjection; using System; @@ -52,5 +53,83 @@ public async Task Can_Resolve_From_ServiceProvider() { IUserService result = await task; Assert.NotNull(result); } + + [Fact] + public async Task Can_Resolve_From_CastleWindsor() { + + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + // Component.For().ImplementedBy().LifestyleNetTransient(), + Classes.FromThisAssembly().BasedOn().WithServiceAllInterfaces().LifestyleNetStatic() + ); + + IUserService actualUserService; + actualUserService = container.Resolve(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => { + IUserService actualUserService = null; + try { + // resolving (with the underlying Castle Windsor, not using Service Provider) with a lifecycle that has an + // accessor that uses something that is AsyncLocal might be troublesome. + // the custom lifecycle accessor will kicks in, but noone assigns the Current scope (which is uninitialized) + actualUserService = container.Resolve(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + + [Fact] + public async Task Can_Resolve_From_ServiceProvider_cretaed_in_UnsafeQueueUserWorkItem() { + + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + // Component.For().ImplementedBy().LifestyleNetTransient(), + Classes.FromThisAssembly().BasedOn().WithServiceAllInterfaces().LifestyleNetStatic() + ); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => { + IUserService actualUserService = null; + try { + // creating a service provider here will be troublesome too + IServiceProvider sp = f.CreateServiceProvider(container); + + actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } } } From df9dd36e8a8202b06cccefcd76daefff9a825f40 Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Wed, 9 Aug 2023 09:24:59 +0200 Subject: [PATCH 4/9] Added Threadpool.Unsafe... failing tests, added comments on critical files --- .editorconfig | 1 + .../ResolveFromThreadpoolUnsafe.cs | 445 ++++++++++++++++-- .../ExtensionContainerRootScopeAccessor.cs | 2 + .../Scope/ExtensionContainerScopeCache.cs | 2 +- 4 files changed, 412 insertions(+), 38 deletions(-) diff --git a/.editorconfig b/.editorconfig index 67ae502916..eaf2b54d43 100644 --- a/.editorconfig +++ b/.editorconfig @@ -10,3 +10,4 @@ indent_size = 2 [*.cs] indent_style = tab indent_size = 4 +csharp_new_line_before_open_brace = all \ No newline at end of file diff --git a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs index 3aa8469fe1..a601c3b921 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs @@ -7,11 +7,55 @@ using System.Threading.Tasks; using Xunit; -namespace Castle.Windsor.Extensions.DependencyInjection.Tests { - public class ResolveFromThreadpoolUnsafe { +namespace Castle.Windsor.Extensions.DependencyInjection.Tests +{ + public class ResolveFromThreadpoolUnsafe + { + #region Singleton + [Fact] - public async Task Can_Resolve_From_ServiceProvider() { + public async Task Can_Resolve_LifestyleSingleton_From_ServiceProvider() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + [Fact] + public async Task Can_Resolve_LifestyleSingleton_From_WindsorContainer() + { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); var f = new WindsorServiceProviderFactory(container); @@ -26,22 +70,221 @@ public async Task Can_Resolve_From_ServiceProvider() { var actualUserService = sp.GetService(); Assert.NotNull(actualUserService); - /* - ThreadPool.UnsafeQueueUserWorkItem(state => { - // resolving using castle (without scopes) works - var actualUserService = container.Resolve(nameof(UserService)); - Assert.NotNull(actualUserService); + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = container.Resolve(nameof(UserService)); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + + [Fact] + public async Task Can_Resolve_LifestyleNetStatic_From_ServiceProvider() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifeStyle.NetStatic() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + + [Fact] + public async Task Can_Resolve_LifestyleNetStatic_From_WindsorContainer() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifeStyle.NetStatic() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = container.Resolve(nameof(UserService)); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + + #endregion + + #region Scoped + + [Fact] + public async Task Can_Resolve_LifestyleScoped_From_ServiceProvider() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifestyleScoped() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + + [Fact] + public async Task Can_Resolve_LifestyleScoped_From_WindsorContainer() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifestyleScoped() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = container.Resolve(nameof(UserService)); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); }, null); - */ + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + + [Fact] + public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_ServiceProvider() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifeStyle.ScopedToNetServiceScope() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); TaskCompletionSource tcs = new TaskCompletionSource(); - ThreadPool.UnsafeQueueUserWorkItem(state => { - try { + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { var actualUserService = sp.GetService(); Assert.NotNull(actualUserService); } - catch (Exception ex) { + catch (Exception ex) + { tcs.SetException(ex); return; } @@ -55,34 +298,77 @@ public async Task Can_Resolve_From_ServiceProvider() { } [Fact] - public async Task Can_Resolve_From_CastleWindsor() { + public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_WindsorContainer() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifeStyle.ScopedToNetServiceScope() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = container.Resolve(nameof(UserService)); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + + #endregion + + #region Transient + + [Fact] + public async Task Can_Resolve_LifestyleTransient_From_ServiceProvider() + { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); var f = new WindsorServiceProviderFactory(container); f.CreateBuilder(serviceProvider); container.Register( - // Component.For().ImplementedBy().LifestyleNetTransient(), - Classes.FromThisAssembly().BasedOn().WithServiceAllInterfaces().LifestyleNetStatic() - ); + Component.For().ImplementedBy().LifestyleTransient() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); - IUserService actualUserService; - actualUserService = container.Resolve(); + var actualUserService = sp.GetService(); Assert.NotNull(actualUserService); TaskCompletionSource tcs = new TaskCompletionSource(); - ThreadPool.UnsafeQueueUserWorkItem(state => { - IUserService actualUserService = null; - try { - // resolving (with the underlying Castle Windsor, not using Service Provider) with a lifecycle that has an - // accessor that uses something that is AsyncLocal might be troublesome. - // the custom lifecycle accessor will kicks in, but noone assigns the Current scope (which is uninitialized) - actualUserService = container.Resolve(); + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = sp.GetService(); Assert.NotNull(actualUserService); } - catch (Exception ex) { + catch (Exception ex) + { tcs.SetException(ex); return; } @@ -96,30 +382,113 @@ public async Task Can_Resolve_From_CastleWindsor() { } [Fact] - public async Task Can_Resolve_From_ServiceProvider_cretaed_in_UnsafeQueueUserWorkItem() { + public async Task Can_Resolve_LifestyleTransient_From_WindsorContainer() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifestyleTransient() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = container.Resolve(nameof(UserService)); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } + + [Fact] + public async Task Can_Resolve_LifestyleNetTransient_From_ServiceProvider() + { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); var f = new WindsorServiceProviderFactory(container); f.CreateBuilder(serviceProvider); container.Register( - // Component.For().ImplementedBy().LifestyleNetTransient(), - Classes.FromThisAssembly().BasedOn().WithServiceAllInterfaces().LifestyleNetStatic() - ); + Component.For().ImplementedBy().LifestyleNetTransient() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); TaskCompletionSource tcs = new TaskCompletionSource(); - ThreadPool.UnsafeQueueUserWorkItem(state => { - IUserService actualUserService = null; - try { - // creating a service provider here will be troublesome too - IServiceProvider sp = f.CreateServiceProvider(container); + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + } - actualUserService = sp.GetService(); + [Fact] + public async Task Can_Resolve_LifestyleNetTransient_From_WindsorContainer() + { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifestyleNetTransient() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = container.Resolve(nameof(UserService)); Assert.NotNull(actualUserService); } - catch (Exception ex) { + catch (Exception ex) + { tcs.SetException(ex); return; } @@ -131,5 +500,7 @@ public async Task Can_Resolve_From_ServiceProvider_cretaed_in_UnsafeQueueUserWor IUserService result = await task; Assert.NotNull(result); } + + #endregion } } diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs index d6c34e3099..5059ea66a3 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs @@ -20,10 +20,12 @@ namespace Castle.Windsor.Extensions.DependencyInjection.Scope { internal class ExtensionContainerRootScopeAccessor : IScopeAccessor { public ILifetimeScope GetScope(CreationContext context) { + /* if (ExtensionContainerScopeCache.Current == null) { // might be null in threads spawn from Threadpool.UnsafeQueueUserWorkItem return null; } + */ return ExtensionContainerScopeCache.Current.RootScope ?? throw new InvalidOperationException("No root scope available"); } diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs index 1e094869c6..9b7b6d8234 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs @@ -24,7 +24,7 @@ internal static class ExtensionContainerScopeCache /// Thrown when there is no scope available. internal static ExtensionContainerScopeBase Current { - get => current.Value; // ?? throw new InvalidOperationException("No scope available"); + get => current.Value ?? throw new InvalidOperationException("No scope available"); // originally there was no exception set => current.Value = value; } } From 64df1b0998c3aff76dac8f59ccf664a9726548af Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Wed, 9 Aug 2023 10:15:30 +0200 Subject: [PATCH 5/9] Improved tests and comments --- .../ResolveFromThreadpoolUnsafe.cs | 84 ++++++- .../Extensions/WindsorExtensions.cs | 4 + .../RegistrationAdapter.cs | 215 ++++++++---------- .../Scope/ExtensionContainerScopeAccessor.cs | 3 +- .../Scope/ExtensionContainerScopeCache.cs | 3 +- 5 files changed, 183 insertions(+), 126 deletions(-) diff --git a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs index a601c3b921..36a2d339cd 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs @@ -13,6 +13,11 @@ public class ResolveFromThreadpoolUnsafe { #region Singleton + /* + * Singleton tests should never fail, given you have a container instance you should always + * be able to resolve a singleton from it. + */ + [Fact] public async Task Can_Resolve_LifestyleSingleton_From_ServiceProvider() { @@ -51,6 +56,9 @@ public async Task Can_Resolve_LifestyleSingleton_From_ServiceProvider() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } [Fact] @@ -76,7 +84,7 @@ public async Task Can_Resolve_LifestyleSingleton_From_WindsorContainer() { try { - var actualUserService = container.Resolve(nameof(UserService)); + var actualUserService = container.Resolve(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -91,6 +99,9 @@ public async Task Can_Resolve_LifestyleSingleton_From_WindsorContainer() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } [Fact] @@ -131,6 +142,9 @@ public async Task Can_Resolve_LifestyleNetStatic_From_ServiceProvider() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } [Fact] @@ -156,7 +170,7 @@ public async Task Can_Resolve_LifestyleNetStatic_From_WindsorContainer() { try { - var actualUserService = container.Resolve(nameof(UserService)); + var actualUserService = container.Resolve(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -171,12 +185,20 @@ public async Task Can_Resolve_LifestyleNetStatic_From_WindsorContainer() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } #endregion #region Scoped + /* + * Scoped tests might fail if for whatever reason you do not have a current scope + * (like when you run from Threadpool.UnsafeQueueUserWorkItem). + */ + [Fact] public async Task Can_Resolve_LifestyleScoped_From_ServiceProvider() { @@ -215,6 +237,9 @@ public async Task Can_Resolve_LifestyleScoped_From_ServiceProvider() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } [Fact] @@ -240,7 +265,7 @@ public async Task Can_Resolve_LifestyleScoped_From_WindsorContainer() { try { - var actualUserService = container.Resolve(nameof(UserService)); + var actualUserService = container.Resolve(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -255,8 +280,16 @@ public async Task Can_Resolve_LifestyleScoped_From_WindsorContainer() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } + /// + /// This test succeeds because WindsorScopedServiceProvider captured the root scope on creation + /// and forced it to be current before service resolution. + /// Scoped is tied to the rootscope = potential memory leak. + /// [Fact] public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_ServiceProvider() { @@ -295,6 +328,9 @@ public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_ServiceProvi var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } [Fact] @@ -320,7 +356,7 @@ public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_WindsorConta { try { - var actualUserService = container.Resolve(nameof(UserService)); + var actualUserService = container.Resolve(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -335,12 +371,26 @@ public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_WindsorConta var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } #endregion #region Transient + /* + * Transient tests failure is questionable: + * - if you have a container you should be able to resolve transient without a scope, + * but they might be tracked by the container itself (or the IServiceProvider) + * - when windsor container is disposed all transient services are disposed as well + * - when a IServiceProvider is disposed all transient services (created by it) are disposed as well + * - problem is: we have una instance of a windsor container passed on to multiple instances of IServiceProvider + * one solution will be to tie the Transients to a scope, and the scope is tied to service provider + * when both of them are disposed, the transient services are disposed as well + */ + [Fact] public async Task Can_Resolve_LifestyleTransient_From_ServiceProvider() { @@ -379,6 +429,9 @@ public async Task Can_Resolve_LifestyleTransient_From_ServiceProvider() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } [Fact] @@ -404,7 +457,7 @@ public async Task Can_Resolve_LifestyleTransient_From_WindsorContainer() { try { - var actualUserService = container.Resolve(nameof(UserService)); + var actualUserService = container.Resolve(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -419,8 +472,16 @@ public async Task Can_Resolve_LifestyleTransient_From_WindsorContainer() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } + /// + /// This test succeeds because WindsorScopedServiceProvider captured the root scope on creation + /// and forced it to be current before service resolution. + /// Transient is tied to the rootscope = potential memory leak. + /// [Fact] public async Task Can_Resolve_LifestyleNetTransient_From_ServiceProvider() { @@ -459,6 +520,9 @@ public async Task Can_Resolve_LifestyleNetTransient_From_ServiceProvider() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } [Fact] @@ -484,7 +548,7 @@ public async Task Can_Resolve_LifestyleNetTransient_From_WindsorContainer() { try { - var actualUserService = container.Resolve(nameof(UserService)); + var actualUserService = container.Resolve(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -499,8 +563,16 @@ public async Task Can_Resolve_LifestyleNetTransient_From_WindsorContainer() var task = tcs.Task; IUserService result = await task; Assert.NotNull(result); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } #endregion + + /* + * Missing tests: we should also test what happens with injected IServiceProvider (what scope do they get?) + * Injected IServiceProvider might or might not have a scope (it depends on AsyncLocal value). + */ } } diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs index 38d46d34d7..6f38a7ed6f 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs @@ -49,6 +49,10 @@ public static ComponentRegistration LifestyleNetTransient(th /// public static ComponentRegistration NetStatic(this LifestyleGroup lifestyle) where TService : class { + //return lifestyle.Singleton; + // I don't think we need this lifestyle at all, usual Singleton should be good + // also we don't need the whole rootscope thing a normal scope set as current should be enough + // otherwise we should revert to static rootscope return lifestyle .Scoped(); } diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs b/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs index 974c02ae88..b9cdf1045b 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs @@ -12,122 +12,101 @@ // See the License for the specific language governing permissions and // limitations under the License. -namespace Castle.Windsor.Extensions.DependencyInjection -{ - using System; - - using Castle.MicroKernel.Registration; - using Castle.Windsor.Extensions.DependencyInjection.Extensions; - - using Microsoft.Extensions.DependencyInjection; - - internal class RegistrationAdapter - { - public static IRegistration FromOpenGenericServiceDescriptor(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) - { - ComponentRegistration registration = Component.For(service.ServiceType) - .NamedAutomatically(UniqueComponentName(service)); - - if(service.ImplementationType != null) - { - registration = UsingImplementation(registration, service); - } - else - { - throw new System.ArgumentException("Unsupported ServiceDescriptor"); - } - - return ResolveLifestyle(registration, service) - .IsDefault(); - } - - public static IRegistration FromServiceDescriptor(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) - { - var registration = Component.For(service.ServiceType) - .NamedAutomatically(UniqueComponentName(service)); - - if (service.ImplementationFactory != null) - { - registration = UsingFactoryMethod(registration, service); - } - else if (service.ImplementationInstance != null) - { - registration = UsingInstance(registration, service); - } - else if (service.ImplementationType != null) - { - registration = UsingImplementation(registration, service); - } - - return ResolveLifestyle(registration, service) - .IsDefault(); - } - - public static string OriginalComponentName(string uniqueComponentName) - { - if(uniqueComponentName == null) - { - return null; - } - if(!uniqueComponentName.Contains("@")) - { - return uniqueComponentName; - } - return uniqueComponentName.Split('@')[0]; - } - - internal static string UniqueComponentName(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) - { - var result = ""; - if(service.ImplementationType != null) - { - result = service.ImplementationType.FullName; - } - else if(service.ImplementationInstance != null) - { - result = service.ImplementationInstance.GetType().FullName; - } - else - { - result = service.ImplementationFactory.GetType().FullName; - } - result = result + "@" + Guid.NewGuid().ToString(); - - return result; - } - - private static ComponentRegistration UsingFactoryMethod(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class - { - return registration.UsingFactoryMethod((kernel) => { - var serviceProvider = kernel.Resolve(); - return service.ImplementationFactory(serviceProvider) as TService; - }); - } - - private static ComponentRegistration UsingInstance(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class - { - return registration.Instance(service.ImplementationInstance as TService); - } - - private static ComponentRegistration UsingImplementation(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class - { - return registration.ImplementedBy(service.ImplementationType); - } - - private static ComponentRegistration ResolveLifestyle(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class - { - switch(service.Lifetime) - { - case ServiceLifetime.Singleton: - return registration.LifeStyle.NetStatic(); - case ServiceLifetime.Scoped: - return registration.LifeStyle.ScopedToNetServiceScope(); - case ServiceLifetime.Transient: - return registration.LifestyleNetTransient(); - - default: - throw new System.ArgumentException($"Invalid lifetime {service.Lifetime}"); - } - } - } +namespace Castle.Windsor.Extensions.DependencyInjection { + using System; + + using Castle.MicroKernel.Registration; + using Castle.Windsor.Extensions.DependencyInjection.Extensions; + + using Microsoft.Extensions.DependencyInjection; + + internal static class RegistrationAdapter { + public static IRegistration FromOpenGenericServiceDescriptor(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) { + ComponentRegistration registration = Component.For(service.ServiceType) + .NamedAutomatically(UniqueComponentName(service)); + + if (service.ImplementationType != null) { + registration = UsingImplementation(registration, service); + } + else { + throw new System.ArgumentException("Unsupported ServiceDescriptor"); + } + + return ResolveLifestyle(registration, service) + .IsDefault(); + } + + public static IRegistration FromServiceDescriptor(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) { + var registration = Component.For(service.ServiceType) + .NamedAutomatically(UniqueComponentName(service)); + + if (service.ImplementationFactory != null) { + registration = UsingFactoryMethod(registration, service); + } + else if (service.ImplementationInstance != null) { + registration = UsingInstance(registration, service); + } + else if (service.ImplementationType != null) { + registration = UsingImplementation(registration, service); + } + + return ResolveLifestyle(registration, service) + .IsDefault(); + } + + public static string OriginalComponentName(string uniqueComponentName) { + if (uniqueComponentName == null) { + return null; + } + if (!uniqueComponentName.Contains("@")) { + return uniqueComponentName; + } + return uniqueComponentName.Split('@')[0]; + } + + internal static string UniqueComponentName(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) { + var result = ""; + if (service.ImplementationType != null) { + result = service.ImplementationType.FullName; + } + else if (service.ImplementationInstance != null) { + result = service.ImplementationInstance.GetType().FullName; + } + else { + result = service.ImplementationFactory.GetType().FullName; + } + result = result + "@" + Guid.NewGuid().ToString(); + + return result; + } + + private static ComponentRegistration UsingFactoryMethod(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class { + return registration.UsingFactoryMethod((kernel) => { + var serviceProvider = kernel.Resolve(); + return service.ImplementationFactory(serviceProvider) as TService; + }); + } + + private static ComponentRegistration UsingInstance(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class { + return registration.Instance(service.ImplementationInstance as TService); + } + + private static ComponentRegistration UsingImplementation(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class { + return registration.ImplementedBy(service.ImplementationType); + } + + private static ComponentRegistration ResolveLifestyle(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class { + switch (service.Lifetime) { + case ServiceLifetime.Singleton: + return registration.LifeStyle.NetStatic(); + case ServiceLifetime.Scoped: + return registration.LifeStyle.ScopedToNetServiceScope(); + case ServiceLifetime.Transient: + return registration.LifestyleNetTransient(); + + default: + throw new System.ArgumentException($"Invalid lifetime {service.Lifetime}"); + } + } + } } \ No newline at end of file diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeAccessor.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeAccessor.cs index 9042944d75..8eaaa9f1d2 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeAccessor.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeAccessor.cs @@ -16,12 +16,13 @@ namespace Castle.Windsor.Extensions.DependencyInjection.Scope { using Castle.MicroKernel.Context; using Castle.MicroKernel.Lifestyle.Scoped; + using System; internal class ExtensionContainerScopeAccessor : IScopeAccessor { public ILifetimeScope GetScope(CreationContext context) { - return ExtensionContainerScopeCache.Current; + return ExtensionContainerScopeCache.Current ?? throw new InvalidOperationException("No scope available. Did you forget to call IServiceScopeFactory.CreateScope()?"); ; } public void Dispose() diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs index 9b7b6d8234..cc48a80320 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs @@ -24,7 +24,8 @@ internal static class ExtensionContainerScopeCache /// Thrown when there is no scope available. internal static ExtensionContainerScopeBase Current { - get => current.Value ?? throw new InvalidOperationException("No scope available"); // originally there was no exception + // AysncLocal can be null in some cases (like Threadpool.UnsafeQueueUserWorkItem) + get => current.Value; // ?? throw new InvalidOperationException("No scope available. Did you forget to call IServiceScopeFactory.CreateScope()?"); set => current.Value = value; } } From 28ac185f27c61f23a4d07137eb23da17919d87be Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Wed, 9 Aug 2023 11:03:57 +0200 Subject: [PATCH 6/9] Option to map custom NetStatic to Castle Singleton lifestyle. this improve resolution of singleton object that might not need a root scope, the windsor container should be enough to resolve singletons --- .../ResolveFromThreadpoolUnsafe.cs | 39 ++++++++++++++++++- .../Extensions/WindsorExtensions.cs | 9 +++-- .../WindsorDependencyInjectionOptions.cs | 16 ++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 src/Castle.Windsor.Extensions.DependencyInjection/WindsorDependencyInjectionOptions.cs diff --git a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs index 36a2d339cd..7bba535e9b 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs @@ -9,8 +9,41 @@ namespace Castle.Windsor.Extensions.DependencyInjection.Tests { - public class ResolveFromThreadpoolUnsafe + [CollectionDefinition(nameof(DoNotParallelize), DisableParallelization = true)] + public class DoNotParallelize { } + + /// + /// These is the original Castle Windsor Dependency Injection behavior. + /// + public class ResolveFromThreadpoolUnsafe_NetStatic: AbstractResolveFromThreadpoolUnsafe + { + public ResolveFromThreadpoolUnsafe_NetStatic() : base(false) + { + } + } + + /// + /// Mapping NetStatic to usual Singleton lifestyle. + /// + public class ResolveFromThreadpoolUnsafe_Singleton : AbstractResolveFromThreadpoolUnsafe + { + public ResolveFromThreadpoolUnsafe_Singleton() : base(true) + { + } + } + + /// + /// relying on static state (WindsorDependencyInjectionOptions) is not good for tests + /// that might run in parallel, can lead to false positives / negatives. + /// + [Collection(nameof(DoNotParallelize))] + public abstract class AbstractResolveFromThreadpoolUnsafe { + protected AbstractResolveFromThreadpoolUnsafe(bool mapNetStaticToSingleton) + { + WindsorDependencyInjectionOptions.MapNetStaticToSingleton = mapNetStaticToSingleton; + } + #region Singleton /* @@ -147,6 +180,10 @@ public async Task Can_Resolve_LifestyleNetStatic_From_ServiceProvider() container.Dispose(); } + /// + /// This test will Succeed is we use standard Castle Windsor Singleton lifestyle instead of the custom + /// NetStatic lifestyle. + /// [Fact] public async Task Can_Resolve_LifestyleNetStatic_From_WindsorContainer() { diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs index 6f38a7ed6f..5b7b95f9d4 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs @@ -49,10 +49,13 @@ public static ComponentRegistration LifestyleNetTransient(th /// public static ComponentRegistration NetStatic(this LifestyleGroup lifestyle) where TService : class { - //return lifestyle.Singleton; - // I don't think we need this lifestyle at all, usual Singleton should be good - // also we don't need the whole rootscope thing a normal scope set as current should be enough + // I don't think we need this lifestyle at all, usual Singleton should be good enough; + // also we maybe don't need the whole rootscope thing. A normal scope set as current should be enough // otherwise we should revert to static rootscope + if (WindsorDependencyInjectionOptions.MapNetStaticToSingleton) + { + return lifestyle.Singleton; + } return lifestyle .Scoped(); } diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/WindsorDependencyInjectionOptions.cs b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorDependencyInjectionOptions.cs new file mode 100644 index 0000000000..ab1ec62ef7 --- /dev/null +++ b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorDependencyInjectionOptions.cs @@ -0,0 +1,16 @@ +namespace Castle.Windsor.Extensions.DependencyInjection +{ + /// + /// Global settins to change the dependency injection behavior. + /// These settings should be set before the container is created. + /// + public static class WindsorDependencyInjectionOptions + { + /// + /// Map NetStatic lifestyle to Castle Windsor Singleton lifestyle. + /// The whole RootScope handling is disabled. + /// (defaut: false) + /// + public static bool MapNetStaticToSingleton { get; set; } + } +} From a210c2a400aa540bfa9b9ad699a26863df16920f Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Wed, 9 Aug 2023 15:00:41 +0200 Subject: [PATCH 7/9] RootScope AsyncLocal cache can be null when AspNetCore tries to create scopes from Threadpool.UnsafeQueueUserWorkItem --- .../RegistrationAdapter.cs | 216 ++++++++++-------- .../Scope/ExtensionContainerScope.cs | 3 +- 2 files changed, 120 insertions(+), 99 deletions(-) diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs b/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs index b9cdf1045b..ad27dce4e0 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/RegistrationAdapter.cs @@ -12,101 +12,123 @@ // See the License for the specific language governing permissions and // limitations under the License. -namespace Castle.Windsor.Extensions.DependencyInjection { - using System; - - using Castle.MicroKernel.Registration; - using Castle.Windsor.Extensions.DependencyInjection.Extensions; - - using Microsoft.Extensions.DependencyInjection; - - internal static class RegistrationAdapter { - public static IRegistration FromOpenGenericServiceDescriptor(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) { - ComponentRegistration registration = Component.For(service.ServiceType) - .NamedAutomatically(UniqueComponentName(service)); - - if (service.ImplementationType != null) { - registration = UsingImplementation(registration, service); - } - else { - throw new System.ArgumentException("Unsupported ServiceDescriptor"); - } - - return ResolveLifestyle(registration, service) - .IsDefault(); - } - - public static IRegistration FromServiceDescriptor(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) { - var registration = Component.For(service.ServiceType) - .NamedAutomatically(UniqueComponentName(service)); - - if (service.ImplementationFactory != null) { - registration = UsingFactoryMethod(registration, service); - } - else if (service.ImplementationInstance != null) { - registration = UsingInstance(registration, service); - } - else if (service.ImplementationType != null) { - registration = UsingImplementation(registration, service); - } - - return ResolveLifestyle(registration, service) - .IsDefault(); - } - - public static string OriginalComponentName(string uniqueComponentName) { - if (uniqueComponentName == null) { - return null; - } - if (!uniqueComponentName.Contains("@")) { - return uniqueComponentName; - } - return uniqueComponentName.Split('@')[0]; - } - - internal static string UniqueComponentName(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) { - var result = ""; - if (service.ImplementationType != null) { - result = service.ImplementationType.FullName; - } - else if (service.ImplementationInstance != null) { - result = service.ImplementationInstance.GetType().FullName; - } - else { - result = service.ImplementationFactory.GetType().FullName; - } - result = result + "@" + Guid.NewGuid().ToString(); - - return result; - } - - private static ComponentRegistration UsingFactoryMethod(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class { - return registration.UsingFactoryMethod((kernel) => { - var serviceProvider = kernel.Resolve(); - return service.ImplementationFactory(serviceProvider) as TService; - }); - } - - private static ComponentRegistration UsingInstance(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class { - return registration.Instance(service.ImplementationInstance as TService); - } - - private static ComponentRegistration UsingImplementation(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class { - return registration.ImplementedBy(service.ImplementationType); - } - - private static ComponentRegistration ResolveLifestyle(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class { - switch (service.Lifetime) { - case ServiceLifetime.Singleton: - return registration.LifeStyle.NetStatic(); - case ServiceLifetime.Scoped: - return registration.LifeStyle.ScopedToNetServiceScope(); - case ServiceLifetime.Transient: - return registration.LifestyleNetTransient(); - - default: - throw new System.ArgumentException($"Invalid lifetime {service.Lifetime}"); - } - } - } +namespace Castle.Windsor.Extensions.DependencyInjection +{ + using System; + + using Castle.MicroKernel.Registration; + using Castle.Windsor.Extensions.DependencyInjection.Extensions; + + using Microsoft.Extensions.DependencyInjection; + + internal static class RegistrationAdapter + { + public static IRegistration FromOpenGenericServiceDescriptor(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) + { + ComponentRegistration registration = Component.For(service.ServiceType) + .NamedAutomatically(UniqueComponentName(service)); + + if (service.ImplementationType != null) + { + registration = UsingImplementation(registration, service); + } + else + { + throw new System.ArgumentException("Unsupported ServiceDescriptor"); + } + + return ResolveLifestyle(registration, service) + .IsDefault(); + } + + public static IRegistration FromServiceDescriptor(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) + { + var registration = Component.For(service.ServiceType) + .NamedAutomatically(UniqueComponentName(service)); + + if (service.ImplementationFactory != null) + { + registration = UsingFactoryMethod(registration, service); + } + else if (service.ImplementationInstance != null) + { + registration = UsingInstance(registration, service); + } + else if (service.ImplementationType != null) + { + registration = UsingImplementation(registration, service); + } + + return ResolveLifestyle(registration, service) + .IsDefault(); + } + + public static string OriginalComponentName(string uniqueComponentName) + { + if (uniqueComponentName == null) + { + return null; + } + if (!uniqueComponentName.Contains("@")) + { + return uniqueComponentName; + } + return uniqueComponentName.Split('@')[0]; + } + + internal static string UniqueComponentName(Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) + { + var result = ""; + if (service.ImplementationType != null) + { + result = service.ImplementationType.FullName; + } + else if (service.ImplementationInstance != null) + { + result = service.ImplementationInstance.GetType().FullName; + } + else + { + result = service.ImplementationFactory.GetType().FullName; + } + result = result + "@" + Guid.NewGuid().ToString(); + + return result; + } + + private static ComponentRegistration UsingFactoryMethod(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class + { + return registration.UsingFactoryMethod((kernel) => + { + var serviceProvider = kernel.Resolve(); + return service.ImplementationFactory(serviceProvider) as TService; + }); + } + + private static ComponentRegistration UsingInstance(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class + { + return registration.Instance(service.ImplementationInstance as TService); + } + + private static ComponentRegistration UsingImplementation(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class + { + return registration.ImplementedBy(service.ImplementationType); + } + + private static ComponentRegistration ResolveLifestyle(ComponentRegistration registration, Microsoft.Extensions.DependencyInjection.ServiceDescriptor service) where TService : class + { + switch (service.Lifetime) + { + case ServiceLifetime.Singleton: + return registration.LifeStyle.NetStatic(); + case ServiceLifetime.Scoped: + return registration.LifeStyle.ScopedToNetServiceScope(); + case ServiceLifetime.Transient: + return registration.LifestyleNetTransient(); + + default: + throw new System.ArgumentException($"Invalid lifetime {service.Lifetime}"); + } + } + } } \ No newline at end of file diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScope.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScope.cs index 2ff5a5c497..9ad99907f7 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScope.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScope.cs @@ -25,10 +25,9 @@ protected ExtensionContainerScope() internal override ExtensionContainerScopeBase RootScope { get; set; } - internal static ExtensionContainerScopeBase BeginScope() { - var scope = new ExtensionContainerScope { RootScope = ExtensionContainerScopeCache.Current.RootScope }; + var scope = new ExtensionContainerScope { RootScope = ExtensionContainerScopeCache.Current?.RootScope }; ExtensionContainerScopeCache.Current = scope; return scope; } From b390405fc371b12396c8694edc3b89155fd333ea Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Thu, 10 Aug 2023 10:53:22 +0200 Subject: [PATCH 8/9] Improved Tests showing supported, unsupported and memory leaks scenarios --- .../ResolveFromThreadpoolUnsafe.cs | 318 +++++++++++++----- .../ExtensionContainerRootScopeAccessor.cs | 2 +- 2 files changed, 231 insertions(+), 89 deletions(-) diff --git a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs index 7bba535e9b..f8d2ce57a4 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection.Tests/ResolveFromThreadpoolUnsafe.cs @@ -1,4 +1,5 @@ -using Castle.MicroKernel.Registration; +using Castle.MicroKernel.Lifestyle; +using Castle.MicroKernel.Registration; using Castle.Windsor.Extensions.DependencyInjection.Extensions; using Castle.Windsor.Extensions.DependencyInjection.Tests.Components; using Microsoft.Extensions.DependencyInjection; @@ -15,44 +16,90 @@ public class DoNotParallelize { } /// /// These is the original Castle Windsor Dependency Injection behavior. /// - public class ResolveFromThreadpoolUnsafe_NetStatic: AbstractResolveFromThreadpoolUnsafe + public class ResolveFromThreadpoolUnsafe_NetStatic : AbstractResolveFromThreadpoolUnsafe { public ResolveFromThreadpoolUnsafe_NetStatic() : base(false) { } - } - /// - /// Mapping NetStatic to usual Singleton lifestyle. - /// - public class ResolveFromThreadpoolUnsafe_Singleton : AbstractResolveFromThreadpoolUnsafe - { - public ResolveFromThreadpoolUnsafe_Singleton() : base(true) + #region "Singleton" + + /// + /// This test will Succeed is we use standard Castle Windsor Singleton lifestyle instead of the custom + /// NetStatic lifestyle. + /// + [Fact] + public async Task Cannot_Resolve_LifestyleNetStatic_From_WindsorContainer_NoRootScopeAvailable() { + var serviceProvider = new ServiceCollection(); + var container = new WindsorContainer(); + var f = new WindsorServiceProviderFactory(container); + f.CreateBuilder(serviceProvider); + + container.Register( + Component.For().ImplementedBy().LifeStyle.NetStatic() + ); + + IServiceProvider sp = f.CreateServiceProvider(container); + + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + + TaskCompletionSource tcs = new TaskCompletionSource(); + + ThreadPool.UnsafeQueueUserWorkItem(state => + { + try + { + var actualUserService = container.Resolve(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var ex = await Catches.ExceptionAsync(async () => + { + var task = tcs.Task; + IUserService result = await task; + // The test succeeds if we use standard Castle Windsor Singleton lifestyle instead of the custom NetStatic lifestyle. + Assert.NotNull(result); + }); + + // This test will fail if we use NetStatic lifestyle + Assert.NotNull(ex); + Assert.IsType(ex); + Assert.Equal("No root scope available.", ex.Message); + + (sp as IDisposable)?.Dispose(); + container.Dispose(); } + + #endregion } /// - /// relying on static state (WindsorDependencyInjectionOptions) is not good for tests - /// that might run in parallel, can lead to false positives / negatives. + /// Mapping NetStatic to usual Singleton lifestyle. /// - [Collection(nameof(DoNotParallelize))] - public abstract class AbstractResolveFromThreadpoolUnsafe + public class ResolveFromThreadpoolUnsafe_Singleton : AbstractResolveFromThreadpoolUnsafe { - protected AbstractResolveFromThreadpoolUnsafe(bool mapNetStaticToSingleton) + public ResolveFromThreadpoolUnsafe_Singleton() : base(true) { - WindsorDependencyInjectionOptions.MapNetStaticToSingleton = mapNetStaticToSingleton; } - #region Singleton - - /* - * Singleton tests should never fail, given you have a container instance you should always - * be able to resolve a singleton from it. - */ + #region "Singleton" + /// + /// This test will Succeed is we use standard Castle Windsor Singleton lifestyle instead of the custom + /// NetStatic lifestyle. + /// [Fact] - public async Task Can_Resolve_LifestyleSingleton_From_ServiceProvider() + public async Task Can_Resolve_LifestyleNetStatic_From_WindsorContainer() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -60,7 +107,7 @@ public async Task Can_Resolve_LifestyleSingleton_From_ServiceProvider() f.CreateBuilder(serviceProvider); container.Register( - Component.For().ImplementedBy() + Component.For().ImplementedBy().LifeStyle.NetStatic() ); IServiceProvider sp = f.CreateServiceProvider(container); @@ -74,7 +121,7 @@ public async Task Can_Resolve_LifestyleSingleton_From_ServiceProvider() { try { - var actualUserService = sp.GetService(); + var actualUserService = container.Resolve(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -86,16 +133,42 @@ public async Task Can_Resolve_LifestyleSingleton_From_ServiceProvider() }, null); // Wait for the work item to complete. - var task = tcs.Task; - IUserService result = await task; - Assert.NotNull(result); + var ex = await Catches.ExceptionAsync(async () => + { + var task = tcs.Task; + IUserService result = await task; + // The test succeeds if we use standard Castle Windsor Singleton lifestyle instead of the custom NetStatic lifestyle. + Assert.NotNull(result); + }); (sp as IDisposable)?.Dispose(); container.Dispose(); } + #endregion + } + + /// + /// relying on static state (WindsorDependencyInjectionOptions) is not good for tests + /// that might run in parallel, can lead to false positives / negatives. + /// + [Collection(nameof(DoNotParallelize))] + public abstract class AbstractResolveFromThreadpoolUnsafe + { + protected AbstractResolveFromThreadpoolUnsafe(bool mapNetStaticToSingleton) + { + WindsorDependencyInjectionOptions.MapNetStaticToSingleton = mapNetStaticToSingleton; + } + + #region Singleton + + /* + * Singleton tests should never fail, given you have a container instance you should always + * be able to resolve a singleton from it. + */ + [Fact] - public async Task Can_Resolve_LifestyleSingleton_From_WindsorContainer() + public async Task Can_Resolve_LifestyleSingleton_From_ServiceProvider() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -117,7 +190,7 @@ public async Task Can_Resolve_LifestyleSingleton_From_WindsorContainer() { try { - var actualUserService = container.Resolve(); + var actualUserService = sp.GetService(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -138,7 +211,7 @@ public async Task Can_Resolve_LifestyleSingleton_From_WindsorContainer() } [Fact] - public async Task Can_Resolve_LifestyleNetStatic_From_ServiceProvider() + public async Task Can_Resolve_LifestyleSingleton_From_WindsorContainer() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -146,7 +219,7 @@ public async Task Can_Resolve_LifestyleNetStatic_From_ServiceProvider() f.CreateBuilder(serviceProvider); container.Register( - Component.For().ImplementedBy().LifeStyle.NetStatic() + Component.For().ImplementedBy() ); IServiceProvider sp = f.CreateServiceProvider(container); @@ -160,7 +233,7 @@ public async Task Can_Resolve_LifestyleNetStatic_From_ServiceProvider() { try { - var actualUserService = sp.GetService(); + var actualUserService = container.Resolve(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -180,12 +253,8 @@ public async Task Can_Resolve_LifestyleNetStatic_From_ServiceProvider() container.Dispose(); } - /// - /// This test will Succeed is we use standard Castle Windsor Singleton lifestyle instead of the custom - /// NetStatic lifestyle. - /// [Fact] - public async Task Can_Resolve_LifestyleNetStatic_From_WindsorContainer() + public async Task Can_Resolve_LifestyleNetStatic_From_ServiceProvider() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -207,7 +276,7 @@ public async Task Can_Resolve_LifestyleNetStatic_From_WindsorContainer() { try { - var actualUserService = container.Resolve(); + var actualUserService = sp.GetService(); Assert.NotNull(actualUserService); } catch (Exception ex) @@ -236,8 +305,12 @@ public async Task Can_Resolve_LifestyleNetStatic_From_WindsorContainer() * (like when you run from Threadpool.UnsafeQueueUserWorkItem). */ + /// + /// This test will fail because the service provider adapter + /// does not create a standard Castle Windsor scope + /// [Fact] - public async Task Can_Resolve_LifestyleScoped_From_ServiceProvider() + public async Task Cannot_Resolve_LifestyleScoped_From_ServiceProvider() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -250,37 +323,52 @@ public async Task Can_Resolve_LifestyleScoped_From_ServiceProvider() IServiceProvider sp = f.CreateServiceProvider(container); - var actualUserService = sp.GetService(); - Assert.NotNull(actualUserService); + // must create a standard Castle Windsor scope (not managed by the adapter) + using (var s = container.BeginScope()) + { + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); - TaskCompletionSource tcs = new TaskCompletionSource(); + TaskCompletionSource tcs = new TaskCompletionSource(); - ThreadPool.UnsafeQueueUserWorkItem(state => - { - try + ThreadPool.UnsafeQueueUserWorkItem(state => { - var actualUserService = sp.GetService(); - Assert.NotNull(actualUserService); - } - catch (Exception ex) + try + { + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var ex = await Catches.ExceptionAsync(async () => { - tcs.SetException(ex); - return; - } - tcs.SetResult(actualUserService); - }, null); + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + }); - // Wait for the work item to complete. - var task = tcs.Task; - IUserService result = await task; - Assert.NotNull(result); + Assert.NotNull(ex); + Assert.IsType(ex); + Assert.StartsWith("Scope was not available. Did you forget to call container.BeginScope()?", ex.Message); + } (sp as IDisposable)?.Dispose(); container.Dispose(); } + /// + /// This test will fail because the service provider adapter + /// does not create a standard Castle Windsor scope + /// [Fact] - public async Task Can_Resolve_LifestyleScoped_From_WindsorContainer() + public async Task Cannot_Resolve_LifestyleScoped_From_WindsorContainer() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -293,30 +381,41 @@ public async Task Can_Resolve_LifestyleScoped_From_WindsorContainer() IServiceProvider sp = f.CreateServiceProvider(container); - var actualUserService = sp.GetService(); - Assert.NotNull(actualUserService); + // must create a standard Castle Windsor scope (not managed by the adapter) + using (var s = container.BeginScope()) + { + var actualUserService = sp.GetService(); + Assert.NotNull(actualUserService); - TaskCompletionSource tcs = new TaskCompletionSource(); + TaskCompletionSource tcs = new TaskCompletionSource(); - ThreadPool.UnsafeQueueUserWorkItem(state => - { - try + ThreadPool.UnsafeQueueUserWorkItem(state => { - var actualUserService = container.Resolve(); - Assert.NotNull(actualUserService); - } - catch (Exception ex) + try + { + var actualUserService = container.Resolve(); + Assert.NotNull(actualUserService); + } + catch (Exception ex) + { + tcs.SetException(ex); + return; + } + tcs.SetResult(actualUserService); + }, null); + + // Wait for the work item to complete. + var ex = await Catches.ExceptionAsync(async () => { - tcs.SetException(ex); - return; - } - tcs.SetResult(actualUserService); - }, null); + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + }); - // Wait for the work item to complete. - var task = tcs.Task; - IUserService result = await task; - Assert.NotNull(result); + Assert.NotNull(ex); + Assert.IsType(ex); + Assert.StartsWith("Scope was not available. Did you forget to call container.BeginScope()?", ex.Message); + } (sp as IDisposable)?.Dispose(); container.Dispose(); @@ -328,7 +427,7 @@ public async Task Can_Resolve_LifestyleScoped_From_WindsorContainer() /// Scoped is tied to the rootscope = potential memory leak. /// [Fact] - public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_ServiceProvider() + public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_ServiceProvider_MemoryLeak() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -371,7 +470,7 @@ public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_ServiceProvi } [Fact] - public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_WindsorContainer() + public async Task Cannot_Resolve_LifestyleScopedToNetServiceScope_From_WindsorContainer() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -405,9 +504,16 @@ public async Task Can_Resolve_LifestyleScopedToNetServiceScope_From_WindsorConta }, null); // Wait for the work item to complete. - var task = tcs.Task; - IUserService result = await task; - Assert.NotNull(result); + var ex = await Catches.ExceptionAsync(async () => + { + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + }); + + Assert.NotNull(ex); + Assert.IsType(ex); + Assert.StartsWith("No scope available", ex.Message); (sp as IDisposable)?.Dispose(); container.Dispose(); @@ -520,7 +626,7 @@ public async Task Can_Resolve_LifestyleTransient_From_WindsorContainer() /// Transient is tied to the rootscope = potential memory leak. /// [Fact] - public async Task Can_Resolve_LifestyleNetTransient_From_ServiceProvider() + public async Task Can_Resolve_LifestyleNetTransient_From_ServiceProvider_MemoryLeak() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -563,7 +669,7 @@ public async Task Can_Resolve_LifestyleNetTransient_From_ServiceProvider() } [Fact] - public async Task Can_Resolve_LifestyleNetTransient_From_WindsorContainer() + public async Task Cannot_Resolve_LifestyleNetTransient_From_WindsorContainer_NoScopeAvailable() { var serviceProvider = new ServiceCollection(); var container = new WindsorContainer(); @@ -597,9 +703,16 @@ public async Task Can_Resolve_LifestyleNetTransient_From_WindsorContainer() }, null); // Wait for the work item to complete. - var task = tcs.Task; - IUserService result = await task; - Assert.NotNull(result); + var ex = await Catches.ExceptionAsync(async () => + { + var task = tcs.Task; + IUserService result = await task; + Assert.NotNull(result); + }); + + Assert.NotNull(ex); + Assert.IsType(ex); + Assert.StartsWith("No scope available", ex.Message); (sp as IDisposable)?.Dispose(); container.Dispose(); @@ -612,4 +725,33 @@ public async Task Can_Resolve_LifestyleNetTransient_From_WindsorContainer() * Injected IServiceProvider might or might not have a scope (it depends on AsyncLocal value). */ } + + public static class Catches + { + public static Exception Exception(Action action) + { + try + { + action(); + } + catch (Exception e) + { + return e; + } + return null; + } + + public async static Task ExceptionAsync(Func func) + { + try + { + await func(); + } + catch (Exception e) + { + return e; + } + return null; + } + } } diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs index 5059ea66a3..62dc0d7638 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs @@ -26,7 +26,7 @@ public ILifetimeScope GetScope(CreationContext context) { return null; } */ - return ExtensionContainerScopeCache.Current.RootScope ?? throw new InvalidOperationException("No root scope available"); + return ExtensionContainerScopeCache.Current?.RootScope ?? throw new InvalidOperationException("No root scope available."); } public void Dispose() { From b997da02488af155cf7f29b0c904c82add359aac Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Thu, 10 Aug 2023 11:42:53 +0200 Subject: [PATCH 9/9] Added comments on WindsorScopedServiceProvider dispose --- .../WindsorScopedServiceProvider.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs index 4e61f2f765..006c475324 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs @@ -18,7 +18,7 @@ namespace Castle.Windsor.Extensions.DependencyInjection using System; using System.Collections.Generic; using System.Reflection; - + using Castle.Windsor; using Castle.Windsor.Extensions.DependencyInjection.Scope; @@ -30,7 +30,7 @@ internal class WindsorScopedServiceProvider : IServiceProvider, ISupportRequired private bool disposing; private readonly IWindsorContainer container; - + public WindsorScopedServiceProvider(IWindsorContainer container) { this.container = container; @@ -39,27 +39,30 @@ public WindsorScopedServiceProvider(IWindsorContainer container) public object GetService(Type serviceType) { - using(_ = new ForcedScope(scope)) + using (_ = new ForcedScope(scope)) { - return ResolveInstanceOrNull(serviceType, true); + return ResolveInstanceOrNull(serviceType, true); } } public object GetRequiredService(Type serviceType) { - using(_ = new ForcedScope(scope)) + using (_ = new ForcedScope(scope)) { - return ResolveInstanceOrNull(serviceType, false); + return ResolveInstanceOrNull(serviceType, false); } } public void Dispose() { + // root scope should be tied to the root IserviceProvider, so + // it has to be disposed with the IserviceProvider to which is tied to if (!(scope is ExtensionContainerRootScope)) return; if (disposing) return; disposing = true; var disposableScope = scope as IDisposable; disposableScope?.Dispose(); + // disping the container here is questionable... what if I want to create another IServiceProvider form the factory? container.Dispose(); } private object ResolveInstanceOrNull(Type serviceType, bool isOptional)