Any ideas on how to write a static analysis rule (FXCop) to ensure that event delegates are removed

Question

We have been going through a big memory leak analysis and have found one of the contributing factors has been the non removal of delegates on events causing objects to not be GCed quickly enough (or sometimes forever).

Would anyone have any ideas as to how to write a rule in FXCop to ensure that we have delegates are removed from handlers?

I've just seen this and as such I'll ask there for more information.

Answer 1

Ok, beside the problem of implementing the actual check (in my opinion this is very similar to a path coverage and thus not practical) - here is the way to write a new FxCop rule it:

At first some articles that helped me once:

• Writing Your Own Custom Rules

• Three Vital FXCop Rules

Implementing a simple rule is no big deal. In your project you need a Rules.xml file as an embedded resource (see here). You derive your class from BaseIntrospectionRule and add your code to the Check()-method:

public override ProblemCollection Check( TypeNode typeNode )
{
  if( type.IsPublic )
  {
    Problems.Add( new Problem( ... ) );
  }
  return Problems;
}

I did this some times ago. I hope it still works as described :)

Answer 2

You need to be more specific. You don't need to check that ALL event delegates were unsubscribed, because in a common case a subscriber lives shorter life than a publisher. And a memory leak only happens when your subscriber appears to be longer-lived than a publisher, hence there is a reference, which prevent GC from collecting the publisher object.

Now we need to verify that if you subscribe to an event on a relatively short-living object, you unsubscribe from it eventually.

An heuristics I can come up with in this case: analyze all local-variable objects (that are scoped by the current code block {}) and all objects, which you explicitly Dispose. For every event on these objects count the number of times you subscribe to them and the number of times you unsubscribe. If the first number is greater then emit a warning.

Of course that doesn't cover all the cases, but I guess no static approach can cover all the cases in this problem, you need some method that is good enough.

I won't mention the advantages of dynamic analysis and code reviews here as it is a separate topic, not related to the question.

Answer 3

Can you force a rule that all event subscriptions should be handled through WeakReferences? I'm thinking this should be easier to implement than having to analyze the actual flow of your program.

Answer 4

Is it safe to assume the objects implementing the handlers somehow have references back to the object with the events? If that's the case you're better off figuring out how to break the cycle another way.

We had something similar going on a while back with event handlers on ASP.NET pages. The objects that implemented the handlers also had references to the pages. After breaking as many of the links architectually as we could, the few left-overs were changed to WeakReferences. No more memory problems!