Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Coverity, Enumerable.Where(this ...), and IDisposable

So code analysis is telling me that Enumarble.Where(this ...) is returning an instance of WhereListIterator<T>, which is (looks to be) an internal type within the .NET framework that implements IDisposable.

Coverity doesn't like IDisposable's to go un-disposed, and is therefore suggesting I dispose of said instance. Obviously I can't dispose of the instance without doing some type checking, as Enumerable.Where(this ...) is said to return IEnumerable<T>, which does not ihnerit from IDisposable.

My question is this: Does .NET expect me to dispose of the WhereListIterator<T>, or does the iterator dispose of itself (say, after each enumeration). If I'm not expected to dispose of it, then why is the interface implemented? This leads me to a third, slightly unrelated question: If IDisposable was implemented explicitly, would Coverity (code analysis) still think that I should dispose of it?

Code Example:

var myList = new List<int>{ 1, 2, 3, 4 };

var evenNumbers = myList.Where(x => x % 2 == 0);

foreach(var number in evenNumbers)
{
    Console.WriteLine(number);
}

if(evenNumbers is IDisposable)
{
    ((IDisposable)evenNumbers).Dispose(); // This line will be executed
}
like image 972
cwharris Avatar asked Dec 01 '11 22:12

cwharris


2 Answers

No, you don't need to dispose of it yourself. Note that you can demonstrate this sort of thing without any need for LINQ. In this case I believe WhereListIterator<T> is actually a hand-written class, but an iterator block shows something similar:

using System;
using System.Collections.Generic;

public class Program
{
    static void Main(string[] args)
    {
        var empty = Empty();
        Console.WriteLine(empty is IDisposable); // Prints True
    }

    static IEnumerable<string> Empty()
    {
        yield break;
    }
}

It really implements IDisposable because it implements not just IEnumerable<T> but also IEnumerator<T> as an optimization - the iterable acts as the iterator as well, in the common case where you only iterate once. A foreach loop will implicitly dispose of an IEnumerator<T>, and you don't need to dispose of it unless you iterate anyway.

Basically, you're fine here - although it's a pity that Coverity is warning you about it. (I haven't used Coverity myself, to be honest - I don't know if there's something you can do to tweak its behaviour here.)

like image 161
Jon Skeet Avatar answered Oct 03 '22 22:10

Jon Skeet


if you are not using the foreach loop and using the old way to iterate

var v = new List<int>() { 1,2,3};
var enumerator = v.GetEnumerator();
while (enumerator.MoveNext())
{

    Console.WriteLine(enumerator.Current);
}

then you should call the Dispose method

like image 29
Sleiman Jneidi Avatar answered Oct 03 '22 21:10

Sleiman Jneidi