How to clean up after an improperly closed file object in the Python Standard Library (after an exception)

Question

TL;DR: The Standard Library fails to close a file when an exception is raised. I'm looking for the best way to handle this situation. Feel free to read from the paragraph beginning with "Upon closer inspection of CPython's source code". Also scroll down to the end of the question to grab a self-contained script that reproduces this issue on Windows.

I'm writing a Python package in which I use STL's ConfigParser (2.x) or configparser (3.x) to parse user config file (I will refer to both as ConfigParser since the problem mainly lies in the 2.x implementation). From now on my relevant lines of code on GitHub will be linked when appropriate throughout. ConfigParser.ConfigParser.read(filenames) (used in my code here) raises a ConfigParser.Error exception when the config file is malformed. I have some code in my test suite targeting this situation, using unittest.TestCase.assertRaises(ConfigParser.Error). The malformed config file is properly generated with tempfile.mkstemp (the returned fd is closed with os.close before anything) and I attempt to remove the temp file with os.remove.

The os.remove is where trouble begins. My tests fail on Windows (while working on both OS X and Ubuntu) with Python 2.7 (see this AppVeyor build):

Traceback (most recent call last):
  File "C:\projects\storyboard\tests\test_util.py", line 235, in test_option_reader
    os.remove(malformed_conf_file)
WindowsError: [Error 32] The process cannot access the file because it is being used by another process: 'c:\\users\\appveyor\\appdata\\local\\temp\\1\\storyboard-test-3clysg.conf'

Note that as I said above, malformed_conf_file is generated with tempfile.mkstemp and immediately closed with os.close, so the only time it is opened is when I call ConfigParser.ConfigParser.read([malformed_conf_file]) here inside the unittest.TestCase.assertRaises(ConfigParser.Error) context. So the culprit seems to be the STL rather than my own code.

Upon closer inspection of CPython's source code, I found that ConfigParser.ConfigPaser.read indeed doesn't close the file properly when an exception is raised. The read method from 2.7 (here on CPython's Mercurial) has the following lines:

for filename in filenames:
    try:
        fp = open(filename)
    except IOError:
        continue
    self._read(fp, filename)
    fp.close()
    read_ok.append(filename)

The exception (if any) is raised by self._read(fp, filename), but as you can see, if self._read raises, then fp won't be closed, since fp.close() is only called after self._read returns.

Meanwhile, the read method from 3.4 (here) does not suffer from the same problem since this time they properly embedded file handling in a context:

for filename in filenames:
    try:
        with open(filename, encoding=encoding) as fp:
            self._read(fp, filename)
    except OSError:
        continue
    read_ok.append(filename)

So I think it's pretty clear that the problem is a defect in 2.7's STL. And what's the best way to handle this situation? Specifically:

• Is there anything I can do from my side to close that file?
• Is it worth reporting to bugs.python.org?

For now I guess I'll just add a try .. except OSError .. to that os.remove (any suggestions?).

---

Update: A self-contained script that can be used to reproduce this issue on Windows:

#!/usr/bin/env python2.7
import ConfigParser
import os
import tempfile

def main():
    fd, path = tempfile.mkstemp()
    os.close(fd)
    with open(path, 'w') as f:
        f.write("malformed\n")
    config = ConfigParser.ConfigParser()
    try:
        config.read(path)
    except ConfigParser.Error:
        pass
    os.remove(path)

if __name__ == '__main__':
    main()

When I run it with Python 2.7 interpreter:

Traceback (most recent call last):
  File ".\example.py", line 19, in <module>
    main()
  File ".\example.py", line 16, in main
    os.remove(path)
WindowsError: [Error 32] The process cannot access the file because it is being used by another process: 'c:\\users\\redacted\\appdata\\local\\temp\\tmp07yq2v'

Answer 1

This is an interesting problem. As Lukas Graf noted in a comment, the problem seems to be that the exception traceback object holds a reference to the call frame where the exception was raised. This call frame includes the local variables that existed at that time, one of which is a reference to the open file. So that file object still has a reference to it and is not properly closed.

For your self-contained example, simply removing the try/except ConfigParser.Error "works": the exception about the malformed config file is uncaught and stops the program. However, in your actual application, assertRaises is catching the exception in order to check that it is the one you're testing for. I'm not 100% sure why the traceback persists even after the with assertRaises block, but apparently it does.

For your example, another more promising fix is to change the pass in your except clause to sys.exc_clear():

try:
    config.read(path)
except ConfigParser.Error:
    sys.exc_clear()

This will get rid of the pesky traceback object and allow the file to be closed.

However, it's not clear exactly how to do that in your real application, because the offending except clause there is inside unittest. I think the easiest thing might be to not use assertRaises directly. Instead, write a helper function that does your test, checks for the exception you want, cleans up with the sys.exc_clear() trick, and then raises another custom exception. Then wrap a call to that helper method in assertRaises. This way you gain control of the problematic exception raised by ConfigParser and can properly clean it up (which unittest isn't doing).

Here's a sketch of what I mean:

# in your test method
assertRaises(CleanedUpConfigError, helperMethod, conf_file, malformed_conf_file)

# helper method that you add to your test case class
def helperMethod(self, conf_file, malformed_conf_file):
     gotRightError = False
     try:
          or6 = OptionReader(
               config_files=[conf_file, malformed_conf_file],
               section='sec',
          )
     except ConfigParser.Error:
          gotRightError = True
          sys.exc_clear()
     if gotRightError:
          raise CleanedUpConfigError("Config error was raised and cleaned up!")

I haven't actually tested this, of course, because I don't have the whole unittest thing set up with your code. You might have to tweak it a bit. (Come to think of it, you might not even need exc_clear() if you do this, because since the exception handler is now in a separate function, the traceback should be correctly cleared when helperMethod exits.) However, I think this idea may get you somewhere. Basically you need to make sure the except clause that catches this particular ConfigParser.Error is written by you, so that it can be cleaned up before you try to remove your test file.

Addendum: it seems that if the context manager handles an exception, the traceback is actually stored until the function containing the with block ends, as this example shows:

class CM(object):
    def __init__(self):
        pass

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_value, tb):
        return True

def foo():
    with CM():
        raise ValueError
    print(sys.exc_info())

Even though the with block has ended by the time the print occurs, so the exception handling should be finished, sys.exc_info still returns exception info as if there were an active exception. This is what is happening in your code too: the with assertRaises block causes the traceback to persist all the way to the end of that function, interfering with your os.remove. This seems like buggy behavior, and I notice that it no longer works this way in Python 3 (the print prints (None, None None)), so I imagine this was a wart that was fixed with Python 3.

Based on this, I suspect it may be sufficient to just insert a sys.exc_clear() right before your os.remove (after the with assertRaises block).