Is it good practice to yield from within a context manager?

Question

I recently wrote a method which returned a sequence of open files; in other words, something like this: ``` # this is very much simplified, of course # the actual code returns file-like objects, not necessarily files def _iterdir(self, *path): dr = os.path.join(*path) paths = imap(lambda fn: os.path.join(dr, fn), os.listdir(dr)) return imap(open, paths) ``` Syntactically, I do not expect to have to close the resulting objects if I do something like: ``` for f in _iterdir('/', 'usr'): make_unicorns_from(f) # ! f.close() ``` As a result, I decided to wrap `_iterdir` in a context manager: ``` def iterdir(self, *path): it = self._iterdir(*path) while 1: with it.next() as f: yield f ``` This appears to be working correctly. What I'm interested in is whether doing this is good practice. Will I run into any issues following this pattern (perhaps if exceptions are thrown)?

Accepted Answer

There are two problems I see. One is that if you try to use more than one file at a time, things break:

```
list(iterdir('/', 'usr')) # Doesn't work; they're all closed.
```

The second is unlikely to happen in CPython, but if you have a reference cycle, or if your code is ever run on a different Python implementation, the problem can manifest.

If an exception happens in `make_unicorns_from(f)`:

```
for f in iterdir('/', 'usr'):
    make_unicorns_from(f) # Uh oh, not enough biomass.
```

The file you were using won't be closed until the generator is garbage-collected. At that point, the generator's `close` method will be called, throwing a `GeneratorExit` exception at the point of the last `yield`, and the exception will cause the context manager to close the file.

With CPython's reference counting, this usually happens immediately. However, on a non-reference-counted implementation or in the presence of a reference cycle, the generator might not be collected until a cycle-detecting GC pass is run. This could take a while.

My gut says to leave closing the files to the caller. You can do

```
for f in _iterdir('/', 'usr'):
    with f:
        make_unicorns_from(f)
```

and they'll all be closed promptly, even without a `with` in the generator, and even if an exception is thrown. I don't know whether or not this is actually a better idea than having the generator take charge of closing the files.

Is it good practice to yield from within a context manager?

Solution

Problem