Memory Leak with delegates and workflow foundation
Recently after Load Testing my open source project Dropthings, I encountered a lot of memory leak. I found lots of Workflow Instances and Linq Entities were left in memory and never collected. After profiling the web application using .NET Memory Profiler, it showed the real picture:
It shows you that instances of the several types are being created but not being removed. You see the “New” column has positive value, but the “Remove” column has 0. That means new instances are being created, but not removed. Basically the way you do Memory Profiling is, you take two snapshots. Say you take one snapshot when you first visit your website. Then you do some action on the website that results in allocation of objects. Then you take another snapshot. When you compare both snapshots, you can see how many instances of classes were created between these two snapshots and how many were removed. If they are not equal, then you have leak. Generally in web application many objects are created on every page hit and the end of the request, all those objects are supposed to be released. If they are not released, then we have a problem. But that’s the scenario for desktop applications because in a desktop application, objects can remain in memory until app is closed. But you should know best from the code which objects were supposed to go out of scope and get released.
For beginners, leak means objects are being allocated but not being freed because someone is holding reference to the objects. When objects leak, they remain in memory forever, until the process (or app domain) is closed. So, if you have a leaky website, your website is continuously taking up memory until it runs out of memory on the web server and thus crash. So, memory leak is a bad – it prevents you from running your product for long duration and requires frequent restart of app pool.
So, the above screenshot shows Workflow and Linq related classes are not being removed, and thus leaking. This means somewhere workflow instances are not being released and thus all workflow related objects are remaining. You can see the number is same 48 for all workflow related objects. This is a good indication that, almost every instance of workflow is leaked because there were total 48 workflows created and ran. Moreover it indicates we have a leak from a top Workflow instance level, not in some specific Activity or somewhere deep in the code.
As the workflows use Linq stuff, they held reference to the Linq stuffs and thus the Linq stuffs leaked as well. Sometimes you might be looking for why A is leaking. But you actually end up finding that since B was holding reference to A and B was leaking and thus A was leaking as well. This is sometimes tricky to figure out and you spend a lot of time looking at the wrong direction.
Now let me show you the buggy code:
ManualWorkflowSchedulerService manualScheduler =
workflowRuntime.GetService<ManualWorkflowSchedulerService>(); WorkflowInstance instance = workflowRuntime.CreateWorkflow(workflowType, properties); instance.Start(); EventHandler<WorkflowCompletedEventArgs> completedHandler = null; completedHandler = delegate(object o, WorkflowCompletedEventArgs e) { if (e.WorkflowInstance.InstanceId == instance.InstanceId) // 1. instance { workflowRuntime.WorkflowCompleted -= completedHandler; // 2. terminatedhandler // copy the output parameters in the specified properties dictionary Dictionary<string,object>.Enumerator enumerator =
e.OutputParameters.GetEnumerator(); while( enumerator.MoveNext() ) { KeyValuePair<string,object> pair = enumerator.Current; if( properties.ContainsKey(pair.Key) ) { properties[pair.Key] = pair.Value; } } } }; Exception x = null; EventHandler<WorkflowTerminatedEventArgs> terminatedHandler = null; terminatedHandler = delegate(object o, WorkflowTerminatedEventArgs e) { if (e.WorkflowInstance.InstanceId == instance.InstanceId) // 3. instance { workflowRuntime.WorkflowTerminated -= terminatedHandler; // 4. completeHandler Debug.WriteLine( e.Exception ); x = e.Exception; } }; workflowRuntime.WorkflowCompleted += completedHandler; workflowRuntime.WorkflowTerminated += terminatedHandler; manualScheduler.RunWorkflow(instance.InstanceId);
Can you spot the code where it leaked?
I have numbered the lines in comment where the leak is happening. Here the delegate is acting like a closure and those who are from Javascript background know closure is evil. They leak memory unless very carefully written. Here the delegate keeps a reference to the instance object. So, if somehow delegate is not released, the instance will remain in memory forever and thus leak. Now can you find a situation when the delegate will not be released?
Say the workflow completed. It will fire the completeHandler. But the completeHandler will not release the terminateHandlerThus the . terminateHandler remains in memory and it also holds reference to the instance. So, we have a leaky delegate leaking whatever it is holding onto outside it’s scope. Here the only thing outside the scope if the instance, which it is tried to access from the parent function.
Since the workflow instance is not released, all the properties the workflow and all the activities inside it are holding onto remains in memory. Most of the workflows and activities expose public properties which are Linq Entities. Thus the Linq Entities remain in memory. Now Linq Entities keep a reference to the DataContext from where it is produced. Thus we have DataContext remaining in memory. Moreover, DataContext keeps reference to many internal objects and metadata cacahe, so they remain in memory as well.
So, the correct code is:
ManualWorkflowSchedulerService manualScheduler =
workflowRuntime.GetService<ManualWorkflowSchedulerService>(); WorkflowInstance instance = workflowRuntime.CreateWorkflow(workflowType, properties); instance.Start(); var instanceId = instance.InstanceId; EventHandler<WorkflowCompletedEventArgs> completedHandler = null; completedHandler = delegate(object o, WorkflowCompletedEventArgs e) { if (e.WorkflowInstance.InstanceId == instanceId) // 1. instanceId is a Guid { // copy the output parameters in the specified properties dictionary Dictionary<string,object>.Enumerator enumerator =
e.OutputParameters.GetEnumerator(); while( enumerator.MoveNext() ) { KeyValuePair<string,object> pair = enumerator.Current; if( properties.ContainsKey(pair.Key) ) { properties[pair.Key] = pair.Value; } } } }; Exception x = null; EventHandler<WorkflowTerminatedEventArgs> terminatedHandler = null; terminatedHandler = delegate(object o, WorkflowTerminatedEventArgs e) { if (e.WorkflowInstance.InstanceId == instanceId) // 2. instanceId is a Guid { x = e.Exception; Debug.WriteLine(e.Exception); } }; workflowRuntime.WorkflowCompleted += completedHandler; workflowRuntime.WorkflowTerminated += terminatedHandler; manualScheduler.RunWorkflow(instance.InstanceId); // 3. Both delegates are now released
workflowRuntime.WorkflowTerminated -= terminatedHandler; workflowRuntime.WorkflowCompleted -= completedHandler;
There are two changes – in both delegates, the instanceId variable is passed, instead of the instance. Since instanceId is a Guid, which is a struct type data type, not a class, there’s no issue of referencing. Structs are copied, not referenced. So, they don’t leak memory. Secondly, both delegates are released at the end of the workflow execution, thus releasing both references.
In Dropthings, I am using the famous CallWorkflow Activity by John Flanders, which is widely used to execute one Workflow from another synchronously. There’s a CallWorkflowService class which is responsible for synchronously executing another workflow and that has similar memory leak problem. The original code of the service is as following:
public class CallWorkflowService : WorkflowRuntimeService { #region Methods public void StartWorkflow(Type workflowType,Dictionary<string,object> inparms,
Guid caller,IComparable qn) { WorkflowRuntime wr = this.Runtime; WorkflowInstance wi = wr.CreateWorkflow(workflowType,inparms); wi.Start(); ManualWorkflowSchedulerService ss =
wr.GetService<ManualWorkflowSchedulerService>(); if (ss != null) ss.RunWorkflow(wi.InstanceId); EventHandler<WorkflowCompletedEventArgs> d = null; d = delegate(object o, WorkflowCompletedEventArgs e) { if (e.WorkflowInstance.InstanceId ==wi.InstanceId) { wr.WorkflowCompleted -= d; WorkflowInstance c = wr.GetWorkflow(caller); c.EnqueueItem(qn, e.OutputParameters, null, null); } }; EventHandler<WorkflowTerminatedEventArgs> te = null; te = delegate(object o, WorkflowTerminatedEventArgs e) { if (e.WorkflowInstance.InstanceId == wi.InstanceId) { wr.WorkflowTerminated -= te; WorkflowInstance c = wr.GetWorkflow(caller); c.EnqueueItem(qn, new Exception("Called Workflow Terminated",
e.Exception), null, null); } }; wr.WorkflowCompleted += d; wr.WorkflowTerminated += te; } #endregion Methods }
As you see, it has that same delegate holding reference to instance object problem. Moreover, there’s some queue stuff there, which requires the caller and qn parameter passed to the StartWorkflow function. So, not a straight forward fix.
I tried to rewrite the whole CallWorkflowService so that it does not require two delegates to be created per Workflow. Then I took the delegates out. Thus there’s no chance of closure holding reference to unwanted objects. The result looks like this:
public class CallWorkflowService : WorkflowRuntimeService { #region Fields private EventHandler<WorkflowCompletedEventArgs> _CompletedHandler = null; private EventHandler<WorkflowTerminatedEventArgs> _TerminatedHandler = null; private Dictionary<Guid, WorkflowInfo> _WorkflowQueue =
new Dictionary<Guid, WorkflowInfo>(); #endregion Fields #region Methods public void StartWorkflow(Type workflowType,Dictionary<string,object> inparms,
Guid caller,IComparable qn) { WorkflowRuntime wr = this.Runtime; WorkflowInstance wi = wr.CreateWorkflow(workflowType,inparms); wi.Start(); var instanceId = wi.InstanceId; _WorkflowQueue[instanceId] = new WorkflowInfo { Caller = caller, qn = qn }; ManualWorkflowSchedulerService ss =
wr.GetService<ManualWorkflowSchedulerService>(); if (ss != null) ss.RunWorkflow(wi.InstanceId); } protected override void OnStarted() { base.OnStarted(); if (null == _CompletedHandler) { _CompletedHandler = delegate(object o, WorkflowCompletedEventArgs e) { var instanceId = e.WorkflowInstance.InstanceId; if (_WorkflowQueue.ContainsKey(instanceId)) { WorkflowInfo wf = _WorkflowQueue[instanceId]; WorkflowInstance c = this.Runtime.GetWorkflow(wf.Caller); c.EnqueueItem(wf.qn, e.OutputParameters, null, null); _WorkflowQueue.Remove(instanceId); } }; this.Runtime.WorkflowCompleted += _CompletedHandler; } if (null == _TerminatedHandler) { _TerminatedHandler = delegate(object o, WorkflowTerminatedEventArgs e) { var instanceId = e.WorkflowInstance.InstanceId; if (_WorkflowQueue.ContainsKey(instanceId)) { WorkflowInfo wf = _WorkflowQueue[instanceId]; WorkflowInstance c = this.Runtime.GetWorkflow(wf.Caller); c.EnqueueItem(wf.qn,
new Exception("Called Workflow Terminated", e.Exception),
null, null); _WorkflowQueue.Remove(instanceId); } }; this.Runtime.WorkflowTerminated += _TerminatedHandler; } } protected override void OnStopped() { _WorkflowQueue.Clear(); base.OnStopped(); } #endregion Methods #region Nested Types private struct WorkflowInfo { #region Fields public Guid Caller; public IComparable qn; #endregion Fields } #endregion Nested Types }
After fixing the problem, another Memory Profile result showed the leak is gone:
As you see, the numbers vary, which means there’s no consistent leak. Moreover, looking at the types that remains in memory, they look more like metadata than instances of classes. So, they are basically cached instances of metadata, not instances allocated during workflow execution which are supposed to be freed. So, we solved the memory leak!
Now you know how to write anonymous delegates without leaking memory and how to run workflow without leaking them. Basically, the principle theory is – if you are referencing some outside object from an anonymous delegate, make sure that object is not holding reference to the delegate in some way, may be directly or may be via some child objects of its own. Because then you have a circular reference. If possible, do not try to access objects e.g. instance inside an anonymous delegate that is declared outside the delegate. Try accessing instrinsic data types like int, string, DateTime, Guid etc which are not reference type variables. So, instead of referencing to an object, you should declare local variables e.g. instanceId that gets the value of properties (e.g. instance.InstanceId) from the object and then use those local variables inside the anonymous delegate.
