Best Practice - Workflow and Anonymous Delegates
Best Practice Recommendation
In your Workflow application (more exactly in the host of your workflow application) never use anonymous delegate like that :
AutoResetEvent waitHandle = new AutoResetEvent(false);
.
.
.
workflowRuntime.WorkflowTerminated += delegate(object sender, WorkflowTerminatedEventArgs e)
{
waitHandle.Set();
};
Details
The code above is very common and works correctly at least most of the time... Actually when you do something like that, then after a few iterations (if you always keep the same instance of the Workflow runtime as it should be the case) you will notice the number of AutoResetEvent object always increases until eventually an Out Of Memory exception is raised.
If you take a debugger like Windbg and display the chain of reference for one particular instance of the AutoResetEvent class you will have an output similar to the following:
DOMAIN(00000000002FF7E0):HANDLE(WeakSh):c1580:Root: 00000000029126e8(System.Workflow.Runtime.WorkflowRuntime)->
000000000296e500(System.EventHandler`1[[System.Workflow.Runtime.WorkflowTerminatedEventArgs, System.Workflow.Runtime]])->
0000000002966970(System.Object[])-> 00000000029570e0(System.EventHandler`1[[System.Workflow.Runtime.WorkflowTerminatedEventArgs, System.Workflow.Runtime]])->
0000000002957038 (TestLeakWorkflow.Program+<>c__DisplayClass2) -> 0000000002957050(System.Threading.AutoResetEvent)
If now you dump the instance of TestLeakWorkflow.Program+<>c__DisplayClass2 you will have the following output :
MT Field Offset Type VT Attr Value Name
000007fef70142d0 4000003 8 AutoResetEvent 0 instance 0000000002957050 waitHandle
What does it tell us? This class TestLeakWorkflow.Program+<>c__DisplayClass2 contains one property of type AutoResetEvent. In our case this property has a strong reference to the object of type AutoResetEvent, consequently the final object is still referenced and remains logically memory.
Why that? Well the purpose of this entry is not to give details about anonymous delegate, but basically there is nothing magic with anonymous delegate, and for the delegate function to access to variable which are not in its scope (look closer, the waitHandle object is not in the scope of the delegate function but still it can access to it) a mechanism is needed. For that, the compiler creates an intermediate class with one property per object which is used in the function delegate but which is normally not in its scope, the delegate function is just a member function of this class; it can then easily access to the properties of the class.
What if you use hundred of objects in the anonymous delegate, objects which are normally not in the scope of the delegate? Well, you will have an intermediate class with hundred of attributes. The instances of the class will then reference the real objects...
Why is that a problem? Well, again look closely? How can you get rid of this instance? You have to remove the reference to the delegate but you cannot (not with the syntax used above), it means the intermediary class and more problematically the final object are still referenced and remain in memory.
To better understand the problem, let’s now study the fragment below, this is a quite classical fragment of code that you will often find in multiple blogs (I know it’s from one of this blog that we have got the code that have been implanted in production and on which I have spent hours debugging to understand why the portal was dying after several hours) :
WorkflowInstance instance = workflowRuntime.CreateWorkflow(typeof(MyASPNetSequencialWorkFlow));
instance.Start();
workflowRuntime.WorkflowCompleted += delegate(object o, WorkflowCompletedEventArgs e1)
{
if (e1.WorkflowInstance.InstanceId == instance.InstanceId)
{ ...
Yes you got it, in this case this is the Workflow instance which will be referenced and which will stay in memory. Considering the fact that in a web application host (as well as in Windows service host) the WorkflowRuntime is a long live object, you will for sure have an Out Of Memory exception.
What do then? Well a code like the one below is definitely better :
EventHandler<WorkflowTerminatedEventArgs> terminatedHandler = null;
EventHandler<WorkflowCompletedEventArgs> completedHandler = null;
terminatedHandler = delegate(object sender, WorkflowTerminatedEventArgs e)
{
if (instance.InstanceId == e.WorkflowInstance.InstanceId)
{
Console.WriteLine(e.Exception.Message);
workflowRuntime.WorkflowCompleted -= completedHandler;
workflowRuntime.WorkflowTerminated -= terminatedHandler;
waitHandle.Set();
}
};
workflowRuntime.WorkflowTerminated += terminatedHandler;
completedHandler = delegate(object sender, WorkflowCompletedEventArgs e)
{
if (instance.InstanceId == e.WorkflowInstance.InstanceId)
{
WorkflowInstance b = instance;
workflowRuntime.WorkflowCompleted -= completedHandler;
workflowRuntime.WorkflowTerminated -= terminatedHandler;
waitHandle.Set();
}
};
workflowRuntime.WorkflowCompleted += completedHandler;
You again have to be prudent, you must remove all the handlers, if at the end WorkflowCompleted Handler is executed, WorkflowTerminated handler will never be executed hence the all the handler are removed in both delegates.
Thanks,
Fabrice
Comments
Anonymous
March 22, 2009
Thank you for submitting this cool story - Trackback from DotNetShoutoutAnonymous
March 10, 2012
You have mentioned in your post: "Actually when you do something like that, then after a few iterations (if you always keep the same instance of the Workflow runtime as it should be the case)" I don't undrestand why i should do that? I cache the xaml files but i create a new WorkflowApplication every time i want to resume or create a new workflow instance.