views:

309

answers:

4

I have a LINQ object with an additional method added to it. The class has no disposable properties or methods, but FxCop is raising the error "Types that own disposable fields should be disposable" and referencing that class.

I've reduced the code this far and still receive the error:

partial class WikiPage
{
    public PagePermissionSet GetUserPermissions(Guid? userId) {
        using (WikiTomeDataContext context = new WikiTomeDataContext()) {
            var permissions =
                from wiki in context.Wikis
                from pageTag in context.VirtualWikiPageTags
                select new {};

            return null;
        }
    }
}

However, if I remove EITHER of the from clauses, FxCop stops giving the error:

partial class WikiPage
{
    public PagePermissionSet GetUserPermissions(Guid? userId) {
        using (WikiTomeDataContext context = new WikiTomeDataContext()) {
            var permissions =
                from pageTag in context.VirtualWikiPageTags
                select new {};

            return null;
        }
    }
}

Or

partial class WikiPage
{
    public PagePermissionSet GetUserPermissions(Guid? userId) {
        using (WikiTomeDataContext context = new WikiTomeDataContext()) {
            var permissions =
                from wiki in context.Wikis
                select new {};

            return null;
        }
    }
}

PagePermissionSet is not disposable.

Is this a false positive? Or is the LINQ code somehow generating a disposable field on the class? If it isn't a false positive, FxCop is recommending that I implement the IDisposable interface, but what would I do in the Dispose method?

EDIT: The full FxCop error is:

"Implement IDisposable on 'WikiPage' because it creates members of the following IDisposable types: 'WikiTomeDataContext'. If 'WikiPage' has previously shipped, adding new members that implement IDisposable to this type is considered a breaking change to existing consumers."

Edit 2: This is the disassembled code that raises the error:

public PagePermissionSet GetUserPermissions(Guid? userId)
{
    using (WikiTomeDataContext context = new WikiTomeDataContext())
    {
        ParameterExpression CS$0$0001;
        ParameterExpression CS$0$0003;
        var permissions = context.Wikis.SelectMany(Expression.Lambda<Func<Wiki, IEnumerable<VirtualWikiPageTag>>>(Expression.Property(Expression.Constant(context), (MethodInfo) methodof(WikiTomeDataContext.get_VirtualWikiPageTags)), new ParameterExpression[] { CS$0$0001 = Expression.Parameter(typeof(Wiki), "wiki") }), Expression.Lambda(Expression.New((ConstructorInfo) methodof(<>f__AnonymousType8..ctor), new Expression[0], new MethodInfo[0]), new ParameterExpression[] { CS$0$0001 = Expression.Parameter(typeof(Wiki), "wiki"), CS$0$0003 = Expression.Parameter(typeof(VirtualWikiPageTag), "pageTag") }));
        return null;
    }
}

Edit 3: There does appear to be a closure class containing a reference to the DataContext. Here is its disassembled code:

[CompilerGenerated]
private sealed class <>c__DisplayClass1
{
    // Fields
    public WikiTomeDataContext context;

    // Methods
    public <>c__DisplayClass1();
}
A: 

Your code that gives the error uses WikiDataContext.

Your two examples that do not give an error use WikiTomeDataContext.

Maybe there is some difference between these two that is causing the error.

Shiraz Bhaiji
Good catch, but that doesn't appear to be it. I've edited my question to use consistent examples.
AaronSieb
+3  A: 

My guess is that the two From clauses generate a call to SelectMany with a closure on your data context. The instance of the closure has a field to the datacontext which is causes the FxCop warning. This is nothing to worry about.

There's only one instance of your datacontext, which you clean up via the using block. Because the closure doesn't have a finalizer there's no performance or saftey implication here in the FxCop warning.

Scott Weinstein
That sounds plausible. If that's the case, do I need to take some kind of action to ensure that the DataContext is disposed?
AaronSieb
There's nothing to worry about. There's only 1 instance of your datacontext, which you clean up via the using block. b/c the closure doesn't have a finalizer, there's no perf implication here.
Scott Weinstein
Thanks! I'll move this error to the exclusion list, then.
AaronSieb
+2  A: 

I noticed that this is a partial class. Have you checked the other implementation file for the class and see if it has an IDisposable member that is not being disposed?

I don't think the generated closure is at fault here. Closures are generated with certain attributes that should cause FxCop to ignore warnings like this.

EDIT

Further investigation by the OP showed this to be an issue with an IDisposable field being lifted into a closure.

Unfortunately there isn't a whole lot you can do about this. There is no way to make the closure implement IDisposable. Event if you could there is no way to call IDisposable on the closure instance.

The best way to approach this problem is to rewrite your code in such a way that a disposable value does not get captured in the closure. Disposable fields should always be disposed when they are finished and capturing it in a closure prevents you from doing this.

JaredPar
If I remove the above method, FxCop doesn't generate the error. I've also looked through the disassembled code for the class, and it doesn't appear to have any IDisposable properties of class-level variables.
AaronSieb
@AaronSieb, interesting. Have you examined the generated closure class to see if any of its members implement IDisposable
JaredPar
@JaredPar How would I do that? I'm a little fuzzy on the concept of closures... The disassembled code (via .NET Reflector) has been appended to the end of my question, if that helps.
AaronSieb
@AaronSieb, look at the type WikiPage and there should be several inner types nested inside it. It will had the name DisplayClass with seemingly random characters on either side of the name. This is the "closure class".
JaredPar
@JaredPar The Closure class does exist, does have a field DataContext field, and does not implement IDisposable. I've attached the disassembled code to the end of my question. What do I need to do to properly clean this up?
AaronSieb
@JaredPar Does the closure reference the same in-memory object as the original local variable? In other words, does calling dispose on the original variable also dispose the value in the closure?
AaronSieb
+1  A: 

If you're returning a LINQ query from your method, consumers will iterate over the results using foreach.

When a consumer finishes a foreach loop, it internally calls dispose on the IEnumerable source (in this case, your LINQ query). This will dispose the WikiTomeDataContext.

However, if a consumer made a call to method returning a LINQ query but never iterated over the results, it would appear that enumerable would never be disposed (that is, until the garbage collector cleaned up the object). This would lead to your WikiTomeDataContext not being disposed until garbage collection.

One way you might be able to get around this problem is by calling .ToArray on the result of your LINQ query, call dispose on your context, then return the array.

Judah Himango
That helps. It isn't clear from the example code, but the LINQ IEnumerable isn't going to be returned. If the DataContext gets properly disposed of when the GC runs, that does make me worry a little less.
AaronSieb