fixed: #507;

improving the docs for resolution scope disposal
dadhi · Aug 2, 2022 · 5b653f6 · 5b653f6
1 parent 7fb806c
commit 5b653f6
Show file tree

Hide file tree

Showing 7 changed files with 146 additions and 39 deletions.
diff --git a/docs/DryIoc.Docs/ReuseAndScopes.cs b/docs/DryIoc.Docs/ReuseAndScopes.cs
@@ -18,7 +18,9 @@
   - [Reuse.ScopedTo(name)](#reusescopedtoname)
     - [Reuse.InWebRequest and Reuse.InThread](#reuseinwebrequest-and-reuseinthread)
   - [Reuse.ScopedTo service type](#reusescopedto-service-type)
-    - [Own the resolution scope disposal](#own-the-resolution-scope-disposal)
+    - [Disposing of resolution scope](#disposing-of-resolution-scope)
+      - [Automatic scope disposal](#automatic-scope-disposal)
+      - [Own the resolution scope disposal](#own-the-resolution-scope-disposal)
   - [Setup.UseParentReuse](#setupuseparentreuse)
   - [Reuse lifespan diagnostics](#reuse-lifespan-diagnostics)
   - [Weakly Referenced reused service](#weakly-referenced-reused-service)
@@ -660,11 +662,18 @@ To satisfy the `ScopedTo` reuse we need an open scope somewhere. Here DryIoc wil
 
 **Note:** The code also tells that `Foo` itself will be scoped to its scope. It may be important if you want to access `Foo` recursively from its dependency.
 
-But with a such an automatic scoping we still have a problem: how to dispose the scope.
 
-In order to dispose the scope, DryIoc should somehow track the reference to it - **otherwise we are in the memory leak territory**. 
+### Disposing of resolution scope
 
-This is exactly how it works, the new resolution scope will itself be held by either the upper scope or by the singleton scope. When the upper scope or container with singletons is disposed - the resolutions scope is disposed to.
+#### Automatic scope disposal
+
+Having such an implicit opened scope poses a problem - how to dispose the scope?
+
+In order to dispose the scope DryIoc or the user code (explained later) 
+should track the reference to it otherwise we have an undisposed dangling scope - **which is the bad thing to have**. 
+
+Therefore to avoid dangling scope the resolution scope will be automatically tracked by either the parent scope or by the singleton scope. 
+When the parent scope or container with singletons is disposed - the resolutions scope is disposed too.
 
 ```cs md*/
 class Scoped_to_service_reuse_with_dispose
@@ -697,10 +706,14 @@ class Dependency : IDisposable
 } /*md
 ```
 
-### Own the resolution scope disposal
 
-You else may get in control of disposing the resolution scope by injecting the `IResolverContext` ([automatically provided for you by Container](RulesAndDefaultConventions.md#implicitly-available-services)) 
-which holds the current scope and then dispose it manually.
+#### Own the resolution scope disposal
+
+The scope tracking is not ideal because it postpones the disposal until parent container or parent scope is disposed
+But we may want to dispose it sooner, e.g. together with the service that opened it.
+
+You may control the disposing of the resolution scope by injecting its `IResolverContext` ([automatically provided by Container](RulesAndDefaultConventions.md#implicitly-available-services)) 
+and then dispose it manually.
 
 ```cs md*/
 class Own_the_resolution_scope_disposal
@@ -717,7 +730,7 @@ [Test] public void Example()
 
         var foo = container.Resolve<Foo>();
 
-        // Disposing the foo will dispose its scope and the scoped dependencies down the tree
+        // Disposing the foo will dispose its scope and its scoped dependencies down the tree
         foo.Dispose(); 
 
         Assert.IsTrue(foo.Dep.IsDisposed);

diff --git a/docs/DryIoc.Docs/ReuseAndScopes.md b/docs/DryIoc.Docs/ReuseAndScopes.md
@@ -17,7 +17,9 @@
   - [Reuse.ScopedTo(name)](#reusescopedtoname)
     - [Reuse.InWebRequest and Reuse.InThread](#reuseinwebrequest-and-reuseinthread)
   - [Reuse.ScopedTo service type](#reusescopedto-service-type)
-    - [Own the resolution scope disposal](#own-the-resolution-scope-disposal)
+    - [Disposing of resolution scope](#disposing-of-resolution-scope)
+      - [Automatic scope disposal](#automatic-scope-disposal)
+      - [Own the resolution scope disposal](#own-the-resolution-scope-disposal)
   - [Setup.UseParentReuse](#setupuseparentreuse)
   - [Reuse lifespan diagnostics](#reuse-lifespan-diagnostics)
   - [Weakly Referenced reused service](#weakly-referenced-reused-service)
@@ -656,11 +658,18 @@ var foo container.OpenScope(new ResolutionScopeName(typeof(Foo))).Resolve<Foo>()
 
 **Note:** The code also tells that `Foo` itself will be scoped to its scope. It may be important if you want to access `Foo` recursively from its dependency.
 
-But with a such an automatic scoping we still have a problem: how to dispose the scope.
 
-In order to dispose the scope, DryIoc should somehow track the reference to it - **otherwise we are in the memory leak territory**. 
+### Disposing of resolution scope
 
-This is exactly how it works, the new resolution scope will itself be held by either the upper scope or by the singleton scope. When the upper scope or container with singletons is disposed - the resolutions scope is disposed to.
+#### Automatic scope disposal
+
+Having such an implicit opened scope poses a problem - how to dispose the scope?
+
+In order to dispose the scope DryIoc or the user code (explained later) 
+should track the reference to it otherwise we have an undisposed dangling scope - **which is the bad thing to have**. 
+
+Therefore to avoid dangling scope the resolution scope will be automatically tracked by either the parent scope or by the singleton scope. 
+When the parent scope or container with singletons is disposed - the resolutions scope is disposed to0.
 
 ```cs 
 class Scoped_to_service_reuse_with_dispose
@@ -693,10 +702,14 @@ class Scoped_to_service_reuse_with_dispose
 } 
 ```
 
-### Own the resolution scope disposal
 
-You else may get in control of disposing the resolution scope by injecting the `IResolverContext` ([automatically provided for you by Container](RulesAndDefaultConventions.md#implicitly-available-services)) 
-which holds the current scope and then dispose it manually.
+#### Own the resolution scope disposal
+
+The scope tracking is not ideal because it postpones the disposal until parent container or parent scope is disposed
+But we may want to dispose it sooner, e.g. together with the service that opened it.
+
+You may control the disposing of the resolution scope by injecting its `IResolverContext` ([automatically provided by Container](RulesAndDefaultConventions.md#implicitly-available-services)) 
+and then dispose it manually.
 
 ```cs 
 class Own_the_resolution_scope_disposal
@@ -713,7 +726,7 @@ class Own_the_resolution_scope_disposal
 
         var foo = container.Resolve<Foo>();
 
-        // Disposing the foo will dispose its scope and the scoped dependencies down the tree
+        // Disposing the foo will dispose its scope and its scoped dependencies down the tree
         foo.Dispose(); 
 
         Assert.IsTrue(foo.Dep.IsDisposed);

diff --git a/src/DryIoc/Container.Generated.cs b/src/DryIoc/Container.Generated.cs
@@ -44,7 +44,7 @@ partial class Container
     {
         partial void GetLastGeneratedFactoryID(ref int lastFactoryID)
         {
-            lastFactoryID = 261; // generated: equals to the last used Factory.FactoryID
+            lastFactoryID = 352; // generated: equals to the last used Factory.FactoryID
         }
 
         partial void ResolveGenerated(ref object service, Type serviceType)
@@ -62,7 +62,7 @@ partial void ResolveGenerated(ref object service,
                     requiredServiceType == null &&
                     Equals(preRequestParent, Request.Empty.Push(
                         typeof(IService),
-                        256,
+                        347,
                         typeof(MyService),
                         Reuse.Transient,
                         RequestFlags.IsResolutionCall)))
@@ -92,7 +92,7 @@ internal static object Get_IService_0(IResolverContext r) =>
                     null,
                     Request.Empty.Push(
                         typeof(IService),
-                        256,
+                        347,
                         typeof(MyService),
                         Reuse.Transient,
                         RequestFlags.IsResolutionCall|RequestFlags.StopRecursiveDependencyCheck),
@@ -104,7 +104,7 @@ internal static object Get_IService_0(IResolverContext r) =>
                     null,
                     Request.Empty.Push(
                         typeof(IService),
-                        256,
+                        347,
                         typeof(MyService),
                         Reuse.Transient,
                         RequestFlags.IsResolutionCall|RequestFlags.StopRecursiveDependencyCheck),

diff --git a/src/DryIoc/Container.cs b/src/DryIoc/Container.cs
@@ -4657,11 +4657,24 @@ public static class ResolverContext
         public static Expression GetRootOrSelfExpr(Request request) =>
             request.Reuse is CurrentScopeReuse == false
             && request.DirectParent.IsSingletonOrDependencyOfSingleton
-            && !request.OpensResolutionScope
             && request.Rules.ThrowIfDependencyHasShorterReuseLifespan // see the #378
+            // && !request.OpensResolutionScope
+            && !request.OpensResolutionScopeUpToResolutionCall()
                 ? RootOrSelfExpr
                 : FactoryDelegateCompiler.ResolverContextParamExpr;
 
+        private static bool OpensResolutionScopeUpToResolutionCall(this Request r)
+        {
+            var p = r.DirectParent;
+            while (p != null)
+            {
+                if ((p.Flags & RequestFlags.OpensResolutionScope) != 0)
+                    return true;
+                p = p.DirectParent;
+            }
+            return false;
+        }
+
         /// <summary>Root or the current resolver context (if it is the root).</summary>
         public static readonly Expression RootOrSelfExpr =
             new ResolverContextArgMethodCallExpression(typeof(ResolverContext).GetMethod(nameof(RootOrSelf)));
@@ -9386,6 +9399,8 @@ public static Request Create(IContainer container, ServiceInfo serviceInfo,
 
                 flags |= preResolveParent.Flags & InheritedFlags;
             }
+            else
+                flags |= preResolveParent.Flags; //inherits the OpensResolutionScope flag
 
             var inputArgExprs = inputArgs?.Map(a => Constant(a)); // todo: @check what happens if `a == null`, does the `object` type for is fine
 
@@ -9491,16 +9506,16 @@ internal void DecreaseTrackedDependencyCountForParents(int dependencyCount)
         public bool IsEmpty => DirectParent == null;
 
         /// <summary>Returns true if request is First in First Resolve call.</summary>
-        public bool IsResolutionRoot => !IsEmpty && DirectParent.IsEmpty;
+        public bool IsResolutionRoot => !IsEmpty && DirectParent.IsEmpty; // todo: @perf remove check for empty
 
         /// <summary>Returns true if request is First in Resolve call.</summary>
-        public bool IsResolutionCall => !IsEmpty && (Flags & RequestFlags.IsResolutionCall) != 0;
+        public bool IsResolutionCall => !IsEmpty && (Flags & RequestFlags.IsResolutionCall) != 0; // todo: @perf remove check for empty
 
         /// <summary>Not the root resolution call.</summary>
         public bool IsNestedResolutionCall => IsResolutionCall && !DirectParent.IsEmpty;
 
         /// <summary>Returns true if request is First in First Resolve call.</summary>
-        public bool OpensResolutionScope => !IsEmpty && (DirectParent.Flags & RequestFlags.OpensResolutionScope) != 0;
+        public bool OpensResolutionScope => !IsEmpty && (DirectParent.Flags & RequestFlags.OpensResolutionScope) != 0; // todo: @perf remove check for empty
 
         /// <summary>Checks if the request Or its parent is wrapped in Func. Use `IsDirectlyWrappedInFunc` for the direct Func wrapper.</summary>
         public bool IsWrappedInFunc() => (Flags & RequestFlags.IsWrappedInFunc) != 0;
@@ -10824,6 +10839,8 @@ cacheEntry.Value is Container.ExprCacheOfTransientWithDepCount t && t.Count > 0
             }
 
             // At last, create the object graph with all of the dependencies created and injected
+            // todo: @perf optimize the code path for IResolverContext Wrapper: ResolverContext.GetRootOrSelfExpr - override GetExpressionOrDefault
+            // todo: @perf for IResolverContext no need to check expression cache at all
             serviceExpr = serviceFactory == null
                 ? CreateExpressionOrDefault(request)
                 : CreateExpressionWithWrappedFactory(request, serviceFactory);

diff --git a/...sTests/GHIssue507_Transient_resolve_with_opening_scope_using_factory_func_in_singleton.cs b/...sTests/GHIssue507_Transient_resolve_with_opening_scope_using_factory_func_in_singleton.cs
@@ -9,7 +9,7 @@ public class GHIssue507_Transient_resolve_with_opening_scope_using_factory_func_
         public int Run()
         {
             Test_Simple();
-            // Test_Original_issue();
+            Test_Original_issue();
             return 2;
         }
 
@@ -41,7 +41,7 @@ class Foo2
         {
         }
 
-        [Test, Ignore("fixme")]
+        [Test]
         public void Test_Original_issue()
         {
             var container = new Container(rules => rules.WithTrackingDisposableTransients());