yield return statement inside a using() { } block Disposes before executing

Neil Fenwick picture Neil Fenwick · Oct 8, 2009 · Viewed 15.7k times · Source

I've written my own custom data layer to persist to a specific file and I've abstracted it with a custom DataContext pattern.

This is all based on the .NET 2.0 Framework (given constraints for the target server), so even though some of it might look like LINQ-to-SQL, its not! I've just implemented a similar data pattern.

See example below for example of a situation that I cannot yet explain.

To get all instances of Animal - I do this and it works fine

public static IEnumerable<Animal> GetAllAnimals() {
        AnimalDataContext dataContext = new AnimalDataContext();
            return dataContext.GetAllAnimals();
}

And the implementation of the GetAllAnimals() method in the AnimalDataContext below

public IEnumerable<Animal> GetAllAnimals() {
        foreach (var animalName in AnimalXmlReader.GetNames())
        {
            yield return GetAnimal(animalName);
        }
}

The AnimalDataContext implements IDisposable because I've got an XmlTextReader in there and I want to make sure it gets cleaned up quickly.

Now if I wrap the first call inside a using statement like so

public static IEnumerable<Animal> GetAllAnimals() {
        using(AnimalDataContext dataContext = new AnimalDataContext()) {
            return dataContext.GetAllAnimals();
        }
}

and put a break-point at the first line of the AnimalDataContext.GetAllAnimals() method and another break-point at the first line in the AnimalDataContext.Dispose() method, and execute...

the Dispose() method is called FIRST so that AnimalXmlReader.GetNames() gives "object reference not set to instance of object" exception because AnimalXmlReader has been set to null in the Dispose() ???

Any ideas? I have a hunch that its related to yield return not being allowed to be called inside a try-catch block, which using effectively represents, once compiled...

Answer

SLaks picture SLaks · Oct 8, 2009

When you call GetAllAnimals it doesn't actually execute any code until you enumerate the returned IEnumerable in a foreach loop.

The dataContext is being disposed as soon as the wrapper method returns, before you enumerate the IEnumerable.

The simplest solution would be to make the wrapper method an iterator as well, like this:

public static IEnumerable<Animal> GetAllAnimals() {
    using (AnimalDataContext dataContext = new AnimalDataContext()) {
        foreach (var animalName in dataContext.GetAllAnimals()) {
            yield return GetAnimal(animalName);
        }
    }
}

This way, the using statement will be compiled in the outer iterator, and it will only be disposed when the outer iterator is disposed.

Another solution would be to enumerate the IEnumerable in the wrapper. The simplest way to do that would be to return a List<Animal>, like this:

public static IEnumerable<Animal> GetAllAnimals() {
    using (AnimalDataContext dataContext = new AnimalDataContext()) {
        return new List<Animal>(dataContext.GetAllAnimals());
    }
}

Note that this loses the benefit of deferred execution, so it will get all of the animals even if you don't need them.