Skip to content

Method injection call handler not being released #21

Description

@cgideon1234

We are attempting to upgrade from Unity 2x to Unity 5.8.11. Everything appears to be working except that the memory allocated for CallHandlers associated with a class that uses a TransientLifetimeManager is not be released by Unity.

I have tried registering the class two different ways with the same result:

Activity

  1. ENikS commented on Jan 3, 2019

    @ENikS
    Contributor

    Could it be the same issue?

  2. cgideon1234 commented on Jan 3, 2019

    @cgideon1234
    Author

    I'm not sure why the message posted before I was finished. I will continue here:

    I registered the class in two different ways:
    _unityContainer.RegisterType<ISystemPreferenceRepository, SystemPreferenceRepository>(
    new Interceptor(),
    new InterceptionBehavior());

    or
    _unityContainer.RegisterType< SystemPreferenceRepository>(
    new Interceptor(),
    new InterceptionBehavior());
    _unityContainer.RegisterType<ISystemPreferenceRepository, SystemPreferenceRepository>();

    It didn't have any impact on it.
    Here is an example of a method causing the problem.
    [Memoization(ConfigurationName = LruCachingKeys.PerProcess)]
    public virtual SystemPreference GetSystemPreference(string name, bool ignoreMemoization = false)

    I have tested it by repeatably calling
    var rep1 = _unityContainer.Resolve< ISystemPreferenceRepository>();
    var rec1 = rep1.GetSystemPreference("Current Year");

    I can put a breakpoint on the destructor of the MemoizationCallHandler and then execute
    GC.Collect();
    GC.WaitForPendingFinalizers();

    In Unity 2x, the breakpoints on the call handler will be hit, indicating it is being freed. In 5x, they are not being hit.
    If I dispose of the UnityContainer in 5x, then the breakpoints are being hit, indicating that it is Unity that is holding a reference to the call handlers.

    We have a long running process that typically runs about 8 hours, with a significant number of calls to a variety of repositories, many have which have call handlers associated with them. With the upgrade to 5x, the server ran very low on memory and it took over 26 hours to complete.

    We have the source code to 5.8.11 but have been unable to determine where the reference to the call handler is being stored. Any help would be greatly appreciated. The upgrade to 5x significantly improved performance in resolving objects, but the memory loss has made it unusable for us.

  3. cgideon1234 commented on Jan 3, 2019

    @cgideon1234
    Author

    It is possible it is related to the issue you referenced. Thanks for the very quick response. I will get the latest build and see if that fixes it.

  4. cgideon1234 commented on Jan 4, 2019

    @cgideon1234
    Author

    Unfortunately that did not work. I upgraded to 5.8.13 and am still getting the same results. The call handlers do not appear to be releasing until I dispose of the UnityContainer. I'm testing this in our full application, but I can try to put together a small test app to get to you.

  5. ENikS commented on Jan 4, 2019

    @ENikS
    Contributor

    Please do, otherwise I would have no idea what is wrong.

  6. cgideon1234 commented on Jan 4, 2019

    @cgideon1234
    Author

    I am attaching a zip file with two projects in it - one for 5.8.13 and one using the old Unity 2x. I create three instances of the repository class that has a call handler based attribute. After that I run the garbage collector to see what is being released. In the 2x version, you can see that the MemoizationCallHandler is being released at the same time as the TestRepository class. However, in the 5x version, you can see that the MemoizationCallHandler is not being released until the UnityContainer is disposed. For some reason in 2x there appear to be 2 instances of the MemoizationCallHandler for each TestRepository,

    Unity 2x
    image

    Unity 5.8.13
    image

    Please let me know if you need anything else from me on this. Thank you.
    TestCallHandlers.zip

  7. cgideon1234 commented on Jan 4, 2019

    @cgideon1234
    Author

    Here is what dotMemory is showing. From what debugging I have done, it looks like the _pipelineManager may be storing a reference to the call handler as well. Maybe this will help:
    image

  8. cgideon1234 commented on Jan 4, 2019

    @cgideon1234
    Author

    One more bit of information that may help. After resolving the TestRepository three times and verifying that the 3 instances of the TestRepository were released, I looked at the Unity container and found where the _pipelineManager is holding the references. It is in the Registrations, in the entry for the TestRepository in the Next chain. Here is what I'm seeing in VS. I didn't expand all of the _pipelines, but you can see at the bottom where it is referencing the MemoizationCallHandler:
    image

  9. transferred this issue fromunitycontainer/unityon Jan 8, 2019
  10. ENikS commented on Jan 29, 2019

    @ENikS
    Contributor

    Could you verify if new version of Unity has the same issue?

  11. cgideon1234 commented on Jan 29, 2019

    @cgideon1234
    Author
  12. bwhalversen commented on Aug 27, 2019

    @bwhalversen

    Carl,

    Did upgrading to a newer version resolve this problem for you? I believe I may be experiencing the same problem. I'm on 5.8.12.

  13. bwhalversen commented on Aug 28, 2019

    @bwhalversen

    @cgideon1234 ,

    Did upgrading to a newer version resolve this problem for you? I believe I may be experiencing the same problem. I'm on 5.8.12.

  14. danielp37 commented on Aug 29, 2019

    @danielp37

    @ENikS
    I've been looking into this with my co-worker @bwhalversen. There appears to be a memory leak in the PreBuildUp method of the TypeInterceptionStrategy. On every resolve of the class that has the inteception behavior applied to it, a new "EffectiveInterceptionBehaviorsPolicy" is set in the BuilderContext.Registration class causing a new one of these to be inserted into its linked list each time the class is resolved. It appears that this class is what holds the PipelineBehavior that contains the CallHandlers to be later passed into the proxy and because these are continually getting inserted into the linked list but never removed, it causes a memory leak of all of the unity types involved here as well as the CallHandlers in the user code.

    I'm working on seeing if I can create a unit tests that detects this issue, but I'm not super familiar with the architecture of the unity interception so its going to take me some time to come up with something but I thought I'd mention what we found in case you could understand and fix what is causing the memory leak just from my description of the problem.

    Thanks,

    Dan

  15. danielp37 commented on Aug 29, 2019

    @danielp37

    Some more information. In Unity 4.x, in the TypeInterceptionStrategy PreBuildup method, it makes the following call:

                context.Policies.Set<EffectiveInterceptionBehaviorsPolicy>(
                    new EffectiveInterceptionBehaviorsPolicy { Behaviors = interceptionBehaviors },
                    context.BuildKey);
    

    In Unity 5.x, it makes a similar call:

                var enumerable = interceptionBehaviors as IInterceptionBehavior[] ?? interceptionBehaviors.ToArray();
                context.Registration.Set(typeof(EffectiveInterceptionBehaviorsPolicy), 
                    new EffectiveInterceptionBehaviorsPolicy { Behaviors = enumerable });
    

    In Unity 4.x the implementation of the Set method on the PolicyList was:

          /// <summary>
           /// Sets an individual policy.
           /// </summary>
           /// <param name="policyInterface">The <see cref="Type"/> of the policy.</param>
           /// <param name="policy">The policy to be registered.</param>
           /// <param name="buildKey">The key the policy applies.</param>
           public void Set(Type policyInterface,
                           IBuilderPolicy policy,
                           object buildKey)
           {
               lock (lockObject)
               {
                   Dictionary<PolicyKey, IBuilderPolicy> newPolicies = this.ClonePolicies();
                   newPolicies[new PolicyKey(policyInterface, buildKey)] = policy;
                   this.policies = newPolicies;
               }
           }
    

    While in 5.x it is

            public virtual void Set(Type policyInterface, object policy)
            {
                if (null == Value && null == Key)
                {
                    Key = policyInterface;
                    Value = policy;
                }
                else
                {
                    Next = new LinkedNode<Type, object>
                    {
                        Key = policyInterface,
                        Value = policy,
                        Next = Next
                    };
                }
            }
    

    It looks like the 4.x implementation would have replaced the "EffectiveInterceptionBehaviorsPolicy" in the dictionary it used where in 5.x we are always inserting into the linked list without checking if the policy is already in the list.

  16. danielp37 commented on Aug 30, 2019

    @danielp37

    I've created a pull request with a potential fix for this: unitycontainer/container#175

  17. danielp37 commented on Sep 10, 2019

    @danielp37

    @ENikS Any comment on what I found above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions