Daniel Fortunov


 

 Daniel Fortunov's Adventures in Software Development » The curious incident of CA1001

 1 Comment- Add comment | Back to Software Development Blog Written on 22-Jan-2009 by asqui

I recently came across a curious behaviour in the Visual Studio Code Analysis feature (formerly known as FxCop).

The description for rule CA1001 is "Types that own disposable fields should be disposable". This is intended to highlight when you've forgotten to implement IDisposable on a class that owns other disposable types.

For example, consider a LogWriter class which internally uses a FileStream to keep track of the file it is writing to. The user of this LogWriter should be able to deterministically close the file that it is being written to. The standard way to do this is to implement IDisposable on LogWriter to dispose of the FileStream. If you don't do this, the user of the LogWriter may be at the mercy of garbage collection, and the FileStream Finalizer, for closure of the log file. This is a Bad Thing because it could (for example) unnecessarily prevent that file being opened by someone else

What's odd is that this rule was triggering in a rather unexpected scenario:

using System;

public sealed class CA1001Repro
{
private object notUsed;

public static void Main()
{
using (Disposable disposable = new Disposable())
{
HigherOrder(a => disposable.DoSomething());
}
}

private static void HigherOrder(Action a)
{
throw new NotImplementedException();
}
}

internal class Disposable : IDisposable
{
public void Dispose()
{
throw new System.NotImplementedException();
}

public void DoSomething()
{
throw new NotImplementedException();
}
}

With code analysis rule CA1001 enabled, this generates the following build output:
CA1001 : Microsoft.Design : Implement IDisposable on 'CA1001Repro' because it creates members of the following IDisposable types: 'DisposableClass'.

Odd isn’t it? It’s telling me quite clearly that the class CA1001Repro creates a member of the type DisposableClass but nothing of this sort is true! The only place I create a DisposableClass instance is within the using block in Main(). The only field member of the class is an object called notUsed.

I presume this error is related to the closure class, which is generated by the syntactic sugar of C# anonymous methods, and allows my inline delegate to seamlessly gain access to the disposableClass variable which is in scope. (See also)

With Reflector we can see this generated class, and sure enough it does have a member of type DisposableClass, and it is also not IDisposable:

[CompilerGenerated]
private sealed class <>c__DisplayClass2
{
// Fields
public DisposableClass disposableClass;

// Methods
public <>c__DisplayClass2();
public void b__0(string a);
}

However, if this were the root of the problem then why did CA1001 explicitly name ‘CA1001Repro’ as the class at fault? Why didn’t it name the true culprit, ‘CA1001Repro.<>c__DisplayClass2’?

And as a final twist of weirdness, if you remove (or comment out) the unused field ‘notUsed’, this problem goes away entirely!

I’ve seen this problem both with Visual Studio 2005 and 2008, and I’ve not been able to find it mentioned anywhere as a known bug with Code Analysis.

Download the repro project here and see if you get the same results. Let me know if you can work out what is going on.

CA1001Repro 

Send to a friend

Comments

Leave a Comment









Loading …
  • Server: web2new.webjam.com
  • Total queries:
  • Serialization time: 296ms
  • Execution time: 374ms
  • XSLT time: $$$XSLT$$$ms