Method injection call handler not being released

[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:

[ENikS]

Could it be the same issue?

[cgideon1234]

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.

[cgideon1234]

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.

[cgideon1234]

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.

[ENikS]

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

[cgideon1234]

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

Unity 5.8.13

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

[cgideon1234]

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:

[cgideon1234]

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:

[ENikS]

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

[cgideon1234]

I will get it installed and tested. It may take a couple of days to get to it. Thanks, Carl

[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.

[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.

[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

[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.

[danielp37]

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

[danielp37]

@ENikS Any comment on what I found above?