Tell FxCop another method is calling dispose

Question

Typically when you dispose a private member, you might do the following:

public void Dispose() {
    var localInst = this.privateMember;
    if (localInst != null) {
        localInst.Dispose();
    }
}

The purpose of the local assignment is to avoid a race condition where another thread might assign the private member to be null after the null check. In this case, I don't care if Dispose is called twice on the instance.

I use this pattern all the time so I wrote an extension method to do this:

public static void SafeDispose(this IDisposable disposable)
{
    if (disposable != null)
    {
        // We also know disposable cannot be null here, 
        // even if the original reference is null.
        disposable.Dispose();
    }
}

And now in my class, I can just do this:

public void Dispose() {
    this.privateMember.SafeDispose();
}

Problem is, FxCop has no idea I'm doing this and it gives me the CA2000: Dispose objects before losing scope warning in every case.

I don't want to turn off this rule and I don't want to suppress every case. Is there a way to hint to FxCop that this method is equivalent to Dispose as far as it's concerned?

Answer 1

The short answer is: there's no way to hint that the object is being disposed elsewhere.

A little bit of Reflector-ing (or dotPeek-ing, or whatever) explains why.

FxCop is in C:\Program Files (x86)\Microsoft Visual Studio 10.0\Team Tools\Static Analysis Tools\FxCop. (Adjust accordingly for your OS/VS version combo.) Rules are in the Rules subdirectory.

In the main FxCop folder, open

• Microsoft.VisualStudio.CodeAnalysis.dll
• Microsoft.VisualStudio.CodeAnalysis.Phoenix.dll
• phx.dll

In the Rules folder, open DataflowRules.dll.

In DataflowRules.dll find Phoenix.CodeAnalysis.DataflowRules.DisposeObjectsBeforeLosingScope. That's the actual class that does the evaluation.

Looking at the code in there, you can see two things of interest with respect to your question.

1. It uses a shared service called SharedNeedsDisposedAnalysis.
2. It derives from FunctionBodyRule.

The first item is interesting because SharedNeedsDisposedAnalysis is what determines which symbols need Dispose() called. It's pretty thorough, doing a "walk" through the code to determine what needs to be disposed and what actually gets disposed. It then keeps a table of those things for later use.

The second item is interesting because FunctionBodyRule rules evaluate the body of a single function. There are other rule types, like FunctionCallRule that evaluate things like function call members (e.g., ProvideCorrectArgumentsToFormattingMethods).

The point is, between the potential "miss" in that SharedNeedsDisposedAnalysis service where it may not be recursing through your method to see that things actually are getting disposed and the limitation of FunctionBodyRule not going beyond the function body, it's just not catching your extension.

This is the same reason "guard functions" like Guard.Against<ArgumentNullException>(arg) never get seen as validating the argument before you use it - FxCop will still tell you to check the argument for null even though that's what the "guard function" is doing.

You have basically two options.

1. Exclude issues or turn off the rule. There's no way it's going to do what you want.
2. Create a custom/derived rule that will understand extension methods. Use your custom rule in place of the default rule.

After having written custom FxCop rules myself, I'll let you know I found it... non-trivial. If you do go down that road, while the recommendation out in the world is to use the new Phoenix engine rule style (that's what the current DisposeObjectsBeforeLosingScope uses), I found it easier to understand the older/standard FxCop SDK rules (see FxCopSdk.dll in the main FxCop folder). Reflector will be a huge help in figuring out how to do that since there's pretty much zero doc on it. Look in the other assemblies in the Rules folder to see examples of those.

Answer 2

I am by no means an FxCop expert at all, but does this question about using SuppressMessage answer it? I don't know if decorating your SafeDispose method with the SuppressMessage attribute would cause FxCop to suppress that message on its analysis of the methods that call it, but seems like it's worth a shot.

Don't trust the syntax below, but something like:

[SuppressMessage("Microsoft.Design", "CA2000:Dispose objects before losing scope", Justification = "We just log the exception and return an HTTP code")]
public static void SafeDispose(this IDisposable disposable)

Answer 3

This code analysis rule is a problematic one, for all the reasons Travis outlined. It seems to queue off any "new" operation, and unless the dispose call is close, CA2000 triggers.

Instead of using new, call a method with this in the body:

MyDisposableClass result;
MyDisposableClass temp = null;
try
{
  temp = new MyDisposableClass();
  //do any initialization here
  result = temp;
  temp = null;
}
finally
{
  if (temp != null) temp.Dispose();
}
return result;

What this does is remove any possibility of the initialization causing the object to be unreachable for disposal. In your case, when you "new up" privatemember, you would do it within a method that looks like the above method. After using this pattern, you are of course still responsible for correct disposal, and your extension method is a great way to generalize that null check.

I've found that you can avoid CA2000 while still passing IDisposables around and doing what you want with them - as long as you new them up correctly within a method like the above. Give it a try and let me know if it works for you. Good luck, and good question!

Other fixes for this rule (including this one) are outlined here: CA2000: Dispose objects before losing scope (Microsoft)