55

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...

WerWet
  • 323
  • 5
  • 14
Neil Fenwick
  • 6,106
  • 3
  • 31
  • 38

2 Answers2

64

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.

Jhonny D. Cano -Leftware-
  • 17,663
  • 14
  • 81
  • 103
SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
  • 1
    Thanks SLaks - 2nd option did the trick and its neat - less lines of code, less bugs!. That call is going into an "AnimalDataContextAdapter" that sits in presentation layer for a WebForms project - specifically to enumerate the collection, so no real loss on deferred execution. – Neil Fenwick Oct 08 '09 at 17:10
  • I would abstract this as: Never accept IDisposable parameters in methods that use `yield return` or other deferring execution methods – Jader Dias Apr 19 '11 at 13:20
  • 1
    The using with `yield return` is safe and calls Dispose on your stream [IFF](https://www.google.com/search?q=define%20iff) the collection is enumerated fully. http://blogs.msdn.com/b/dancre/archive/2008/03/14/yield-and-usings-your-dispose-may-not-be-called.aspx – yzorg Aug 07 '15 at 15:00
  • This essentially means I can use yield return ONLY for iterating in-memory collection as remote queries will always use some class to fetch data which usually implement IDisposable. – RBT Jul 01 '16 at 09:16
12

The reason for this is that the GetAllAnimals method doesn't return a colleciton of animals. It returns an enumerator that is capable of returning an animal at a time.

When you return the result from the GetAllAnimals call inside the using block, you just return the enumerator. The using block disposes the data context before the method exits, and at that point the enumerator have not yet read any animals at all. When you then try to use the enumerator, it can not get any animals from the data context.

A workaround is to make the GetAllAnimals method also create an enumerator. That way the using block will not be closed until you stop using that enumerator:

public static IEnumerable<Animal> GetAllAnimals() {
   using(AnimalDataContext dataContext = new AnimalDataContext()) {
      foreach (Animal animal in dataContext.GetAllAnimals()) {
         yield return animal;
      }
   }
}
Guffa
  • 687,336
  • 108
  • 737
  • 1,005