From 464595bb92eba900b0439f495baf1bb6f41eb12b Mon Sep 17 00:00:00 2001 From: Ruben Vandeginste Date: Mon, 7 Aug 2023 00:29:54 +0200 Subject: [PATCH] Fix #646: fix scope and scope-accessor implementation To fix issue #563 (support concurrently existing containers) a fix was created in #577. After the initial fix some refactorings were done on the scope implementation that were not correct. This commit changes the scope implementation back to the original implementation of @ltines, but with the support for concurrent containers. --- .../Extensions/WindsorExtensions.cs | 2 +- .../Scope/ExtensionContainerRootScope.cs | 11 ++- .../ExtensionContainerRootScopeAccessor.cs | 12 ++- .../Scope/ExtensionContainerScope.cs | 91 ++++++++++++++++--- .../Scope/ExtensionContainerScopeAccessor.cs | 8 +- .../Scope/ExtensionContainerScopeBase.cs | 66 -------------- .../Scope/ExtensionContainerScopeCache.cs | 31 ------- .../Scope/ForcedScope.cs | 38 -------- .../{Scope => }/ServiceScope.cs | 21 ++++- .../{Scope => }/WindsorScopeFactory.cs | 17 +++- .../WindsorScopedServiceProvider.cs | 30 +++--- 11 files changed, 149 insertions(+), 178 deletions(-) delete mode 100644 src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeBase.cs delete mode 100644 src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs delete mode 100644 src/Castle.Windsor.Extensions.DependencyInjection/Scope/ForcedScope.cs rename src/Castle.Windsor.Extensions.DependencyInjection/{Scope => }/ServiceScope.cs (67%) rename src/Castle.Windsor.Extensions.DependencyInjection/{Scope => }/WindsorScopeFactory.cs (72%) diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs index 38d46d34d7..38e23c1759 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Extensions/WindsorExtensions.cs @@ -39,7 +39,7 @@ public static ComponentRegistration ScopedToNetServiceScope( public static ComponentRegistration LifestyleNetTransient(this ComponentRegistration registration) where TService : class { return registration - .Attribute(ExtensionContainerScopeBase.TransientMarker).Eq(Boolean.TrueString) + .Attribute(ExtensionContainerScope.TransientMarker).Eq(Boolean.TrueString) .LifeStyle.ScopedToNetServiceScope(); //.NET core expects new instances but release on scope dispose } diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScope.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScope.cs index 95f29a07d3..d2358270b5 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScope.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScope.cs @@ -14,16 +14,17 @@ namespace Castle.Windsor.Extensions.DependencyInjection.Scope { - internal class ExtensionContainerRootScope : ExtensionContainerScopeBase + internal class ExtensionContainerRootScope : ExtensionContainerScope { - + private ExtensionContainerRootScope() : base(null, null) + { + } + public static ExtensionContainerRootScope BeginRootScope() { var scope = new ExtensionContainerRootScope(); - ExtensionContainerScopeCache.Current = scope; + current.Value = scope; return scope; } - - internal override ExtensionContainerScopeBase RootScope => this; } } diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs index 25139efec9..e15b5455e0 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerRootScopeAccessor.cs @@ -23,7 +23,17 @@ internal class ExtensionContainerRootScopeAccessor : IScopeAccessor { public ILifetimeScope GetScope(CreationContext context) { - return ExtensionContainerScopeCache.Current.RootScope ?? throw new InvalidOperationException("No root scope available"); + if (ExtensionContainerScope.Current == null) + { + throw new InvalidOperationException("No root scope"); + } + + if (ExtensionContainerScope.Current.RootScope == null) + { + throw new InvalidOperationException("No root scope"); + } + + return ExtensionContainerScope.Current.RootScope; } public void Dispose() diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScope.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScope.cs index 2ff5a5c497..ead6cec9ed 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScope.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScope.cs @@ -1,4 +1,4 @@ -// Copyright 2004-2020 Castle Project - http://www.castleproject.org/ +// Copyright 2004-2020 Castle Project - http://www.castleproject.org/ // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -14,32 +14,93 @@ namespace Castle.Windsor.Extensions.DependencyInjection.Scope { - internal class ExtensionContainerScope : ExtensionContainerScopeBase + using System; + using System.Threading; + + using Castle.Core; + using Castle.MicroKernel; + using Castle.MicroKernel.Lifestyle.Scoped; + + internal class ExtensionContainerScope : ILifetimeScope, IDisposable { - private readonly ExtensionContainerScopeBase parent; + public static ExtensionContainerScope Current => current.Value; + public static string TransientMarker = "Transient"; + protected static readonly AsyncLocal current = new AsyncLocal(); + private readonly ExtensionContainerScope parent; + private readonly ExtensionContainerRootScope rootScope; + private readonly IScopeCache scopeCache; - protected ExtensionContainerScope() + protected ExtensionContainerScope( + ExtensionContainerScope parent, + ExtensionContainerRootScope rootScope) { - parent = ExtensionContainerScopeCache.Current; + scopeCache = new ScopeCache(); + this.parent = parent ?? rootScope; + this.rootScope = rootScope; } - internal override ExtensionContainerScopeBase RootScope { get; set; } - + public ExtensionContainerRootScope RootScope + => this as ExtensionContainerRootScope ?? rootScope; - internal static ExtensionContainerScopeBase BeginScope() + public static ExtensionContainerScope BeginScope(ExtensionContainerScope parent, ExtensionContainerRootScope rootScope) { - var scope = new ExtensionContainerScope { RootScope = ExtensionContainerScopeCache.Current.RootScope }; - ExtensionContainerScopeCache.Current = scope; + if (rootScope == null) + throw new ArgumentNullException(nameof(rootScope)); + + var scope = new ExtensionContainerScope(parent, rootScope); + current.Value = scope; return scope; } - public override void Dispose() + public void Dispose() + { + var disposableCache = scopeCache as IDisposable; + if (disposableCache != null) + { + disposableCache.Dispose(); + } + + current.Value = parent; + } + + public Burden GetCachedInstance(ComponentModel model, ScopedInstanceActivationCallback createInstance) { - if (ExtensionContainerScopeCache.current.Value == this) + lock (scopeCache) + { + // Add transient's burden to scope so it gets released + if (model.Configuration.Attributes.Get(TransientMarker) == bool.TrueString) + { + var transientBurden = createInstance((_) => {}); + scopeCache[transientBurden] = transientBurden; + return transientBurden; + } + + var scopedBurden = scopeCache[model]; + if (scopedBurden != null) + { + return scopedBurden; + } + scopedBurden = createInstance((_) => {}); + scopeCache[model] = scopedBurden; + return scopedBurden; + } + } + + /// + /// Forces a specific for 'using' block. In .NET scope is tied to an instance of not a thread or async context + /// + internal class ForcedScope : IDisposable + { + private readonly ExtensionContainerScope previousScope; + public ForcedScope(ExtensionContainerScope scope) + { + previousScope = ExtensionContainerScope.Current; + ExtensionContainerScope.current.Value = scope; + } + public void Dispose() { - ExtensionContainerScopeCache.current.Value = parent; + ExtensionContainerScope.current.Value = previousScope; } - base.Dispose(); } } -} \ 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..cf6621fdab 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeAccessor.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeAccessor.cs @@ -14,6 +14,8 @@ namespace Castle.Windsor.Extensions.DependencyInjection.Scope { + using System; + using Castle.MicroKernel.Context; using Castle.MicroKernel.Lifestyle.Scoped; @@ -21,7 +23,11 @@ internal class ExtensionContainerScopeAccessor : IScopeAccessor { public ILifetimeScope GetScope(CreationContext context) { - return ExtensionContainerScopeCache.Current; + if(ExtensionContainerScope.Current == null) + { + throw new InvalidOperationException("No scope available"); + } + return ExtensionContainerScope.Current; } public void Dispose() diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeBase.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeBase.cs deleted file mode 100644 index edb784dfcb..0000000000 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeBase.cs +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2004-2020 Castle Project - http://www.castleproject.org/ -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -namespace Castle.Windsor.Extensions.DependencyInjection.Scope -{ - using System; - - using Castle.Core; - using Castle.MicroKernel; - using Castle.MicroKernel.Lifestyle.Scoped; - - internal abstract class ExtensionContainerScopeBase : ILifetimeScope - { - public static readonly string TransientMarker = "Transient"; - private readonly IScopeCache scopeCache; - - protected ExtensionContainerScopeBase() - { - scopeCache = new ScopeCache(); - } - - internal virtual ExtensionContainerScopeBase RootScope { get; set; } - - public virtual void Dispose() - { - if (scopeCache is IDisposable disposableCache) - { - disposableCache.Dispose(); - } - } - - public Burden GetCachedInstance(ComponentModel model, ScopedInstanceActivationCallback createInstance) - { - lock (scopeCache) - { - // Add transient's burden to scope so it gets released - if (model.Configuration.Attributes.Get(TransientMarker) == bool.TrueString) - { - var transientBurden = createInstance(_ => {}); - scopeCache[transientBurden] = transientBurden; - return transientBurden; - } - - var scopedBurden = scopeCache[model]; - if (scopedBurden != null) - { - return scopedBurden; - } - scopedBurden = createInstance((_) => {}); - scopeCache[model] = scopedBurden; - return scopedBurden; - } - } - } -} \ No newline at end of file diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs deleted file mode 100644 index d89f9dd1f2..0000000000 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ExtensionContainerScopeCache.cs +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2004-2020 Castle Project - http://www.castleproject.org/ -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -namespace Castle.Windsor.Extensions.DependencyInjection.Scope -{ - using System; - using System.Threading; - - internal static class ExtensionContainerScopeCache - { - internal static readonly AsyncLocal current = new AsyncLocal(); - /// Current scope for the thread. Initial scope will be set when calling BeginRootScope from a ExtensionContainerRootScope instance. - /// Thrown when there is no scope available. - internal static ExtensionContainerScopeBase Current - { - get => current.Value ?? throw new InvalidOperationException("No scope available"); - set => current.Value = value; - } - } -} \ No newline at end of file diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ForcedScope.cs b/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ForcedScope.cs deleted file mode 100644 index c9d41dfa66..0000000000 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ForcedScope.cs +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2004-2020 Castle Project - http://www.castleproject.org/ -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -namespace Castle.Windsor.Extensions.DependencyInjection.Scope -{ - using System; - - /// - /// Forces a specific for 'using' block. In .NET scope is tied to an instance of not a thread or async context - /// - internal class ForcedScope : IDisposable - { - private readonly ExtensionContainerScopeBase scope; - private readonly ExtensionContainerScopeBase previousScope; - internal ForcedScope(ExtensionContainerScopeBase scope) - { - previousScope = ExtensionContainerScopeCache.Current; - this.scope = scope; - ExtensionContainerScopeCache.Current = scope; - } - public void Dispose() - { - if(ExtensionContainerScopeCache.Current != scope) return; - ExtensionContainerScopeCache.Current = previousScope; - } - } -} \ No newline at end of file diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ServiceScope.cs b/src/Castle.Windsor.Extensions.DependencyInjection/ServiceScope.cs similarity index 67% rename from src/Castle.Windsor.Extensions.DependencyInjection/Scope/ServiceScope.cs rename to src/Castle.Windsor.Extensions.DependencyInjection/ServiceScope.cs index c033687851..e8a0d24d43 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/ServiceScope.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/ServiceScope.cs @@ -12,23 +12,34 @@ // 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 { using System; - + using Microsoft.Extensions.DependencyInjection; internal class ServiceScope : IServiceScope { private readonly IDisposable scope; + private readonly IServiceProvider serviceProvider; public ServiceScope(IDisposable windsorScope, IServiceProvider serviceProvider) { - scope = windsorScope ?? throw new ArgumentNullException(nameof(scope)); - ServiceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider)); + if(windsorScope == null) + { + throw new ArgumentNullException(nameof(scope)); + } + + if(serviceProvider == null) + { + throw new ArgumentNullException(nameof(serviceProvider)); + } + + this.scope = windsorScope; + this.serviceProvider = serviceProvider; } - public IServiceProvider ServiceProvider { get; } + public IServiceProvider ServiceProvider => serviceProvider; public void Dispose() { diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/WindsorScopeFactory.cs b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopeFactory.cs similarity index 72% rename from src/Castle.Windsor.Extensions.DependencyInjection/Scope/WindsorScopeFactory.cs rename to src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopeFactory.cs index 0686ea95a6..4264ff28fb 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/Scope/WindsorScopeFactory.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopeFactory.cs @@ -13,26 +13,35 @@ // limitations under the License. -namespace Castle.Windsor.Extensions.DependencyInjection.Scope +namespace Castle.Windsor.Extensions.DependencyInjection { using System; using Castle.Windsor; + using Castle.Windsor.Extensions.DependencyInjection.Scope; using Microsoft.Extensions.DependencyInjection; internal class WindsorScopeFactory : IServiceScopeFactory { private readonly IWindsorContainer scopeFactoryContainer; - - public WindsorScopeFactory(IWindsorContainer container) + private readonly ExtensionContainerRootScope rootScope; + + public WindsorScopeFactory( + IWindsorContainer container, + ExtensionContainerRootScope rootScope) { scopeFactoryContainer = container; + this.rootScope = rootScope; } public IServiceScope CreateScope() { - var scope = ExtensionContainerScope.BeginScope(); + var scope = + ExtensionContainerScope + .BeginScope( + ExtensionContainerScope.Current, + rootScope); //since WindsorServiceProvider is scoped, this gives us new instance var provider = scopeFactoryContainer.Resolve(); diff --git a/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs index 4e61f2f765..cbbc331b33 100644 --- a/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs +++ b/src/Castle.Windsor.Extensions.DependencyInjection/WindsorScopedServiceProvider.cs @@ -26,20 +26,20 @@ namespace Castle.Windsor.Extensions.DependencyInjection internal class WindsorScopedServiceProvider : IServiceProvider, ISupportRequiredService, IDisposable { - private readonly ExtensionContainerScopeBase scope; - private bool disposing; + private readonly ExtensionContainerScope scope; + private bool disposing = false; private readonly IWindsorContainer container; public WindsorScopedServiceProvider(IWindsorContainer container) { this.container = container; - scope = ExtensionContainerScopeCache.Current; + this.scope = ExtensionContainerScope.Current; } public object GetService(Type serviceType) { - using(_ = new ForcedScope(scope)) + using(var fs = new ExtensionContainerScope.ForcedScope(scope)) { return ResolveInstanceOrNull(serviceType, true); } @@ -47,7 +47,7 @@ public object GetService(Type serviceType) public object GetRequiredService(Type serviceType) { - using(_ = new ForcedScope(scope)) + using(var fs = new ExtensionContainerScope.ForcedScope(scope)) { return ResolveInstanceOrNull(serviceType, false); } @@ -55,12 +55,20 @@ public object GetRequiredService(Type serviceType) public void Dispose() { - if (!(scope is ExtensionContainerRootScope)) return; - if (disposing) return; - disposing = true; - var disposableScope = scope as IDisposable; - disposableScope?.Dispose(); - container.Dispose(); + if(scope is ExtensionContainerRootScope) + { + if(!disposing) + { + disposing = true; + var disposableScope = scope as IDisposable; + if(disposableScope != null) + { + disposableScope.Dispose(); + } + container.Dispose(); + } + + } } private object ResolveInstanceOrNull(Type serviceType, bool isOptional) {