BUG: read_csv can leak file handles if validation fails

[mirober]

Pandas version checks

• I have checked that this issue has not already been reported.

• I have confirmed this bug exists on the latest version of pandas.

• I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

import pandas as pd
import os

TEST_CONTENTS = """
col1,col2
a,b
1,2
"""
TEST_PATH = "file_leak_example.csv"
with open(TEST_PATH, "w+") as f:
    f.write(TEST_CONTENTS)

try:
    # usecols specifies incorrect columns, causing an Exception during parser init
    pd.read_csv(TEST_PATH, usecols=["col1", "col2", "col3"], engine="c")
except:
    # Fails on Windows, as the file is still open
    os.unlink(TEST_PATH)

# Causes the same problem, due to "a" not being a column in the file
# pd.read_csv(TEST_PATH, engine="python", parse_dates=["a"])

Issue Description

NOTE: I think this will only repro the issue on Windows, as other OSs do not prevent you deleting a file that is still open.

On Windows, the above repro causes the following:

$ python file_leak.py
Traceback (most recent call last):
  File "C:\Users\Mike\Documents\GitHub\pandas\file_leak.py", line 15, in <module>
    pd.read_csv(TEST_PATH, usecols=["col1", "col2", "col3"], engine="c")
  File "C:\Users\Mike\Documents\GitHub\pandas\pandas\util\_decorators.py", line 311, in wrapper
    return func(*args, **kwargs)
  File "C:\Users\Mike\Documents\GitHub\pandas\pandas\io\parsers\readers.py", line 674, in read_csv
    return _read(filepath_or_buffer, kwds)
  File "C:\Users\Mike\Documents\GitHub\pandas\pandas\io\parsers\readers.py", line 569, in _read
    parser = TextFileReader(filepath_or_buffer, **kwds)
  File "C:\Users\Mike\Documents\GitHub\pandas\pandas\io\parsers\readers.py", line 924, in __init__
    self._engine = self._make_engine(self.engine)
  File "C:\Users\Mike\Documents\GitHub\pandas\pandas\io\parsers\readers.py", line 1192, in _make_engine
    return mapping[engine](self.f, **self.options)
  File "C:\Users\Mike\Documents\GitHub\pandas\pandas\io\parsers\c_parser_wrapper.py", line 142, in __init__
    self._validate_usecols_names(usecols, self.orig_names)
  File "C:\Users\Mike\Documents\GitHub\pandas\pandas\io\parsers\base_parser.py", line 941, in _validate_usecols_names
    raise ValueError(
ValueError: Usecols do not match columns, columns expected but not found: ['col3']

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\Mike\Documents\GitHub\pandas\file_leak.py", line 18, in <module>
    os.unlink(TEST_PATH)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'file_leak_example.csv'

The ValueError is correct, but the os.unlink should not fail. The reason it does is due to the csv parser holding onto the file handle.

Expected Behavior

If an exception is encountered during file reading all handles should be closed, allowing invalid files to be moved or deleted.

Installed Versions

In [2]: pd.show_versions()

INSTALLED VERSIONS

commit : ad19057
python : 3.9.5.final.0
python-bits : 64
OS : Windows
OS-release : 10
Version : 10.0.19043
machine : AMD64
processor : Intel64 Family 6 Model 60 Stepping 3, GenuineIntel
byteorder : little
LC_ALL : None
LANG : en_GB.UTF-8
LOCALE : English_United Kingdom.1252

pandas : 1.5.0.dev0+59.gad190575aa
numpy : 1.21.0
pytz : 2021.1
dateutil : 2.8.1
pip : 21.3.1
setuptools : 51.3.3
Cython : 0.29.24
pytest : 6.2.5
hypothesis : 6.35.0
sphinx : None
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : 3.0.1
IPython : 7.25.0
pandas_datareader: None
bs4 : None
bottleneck : None
fsspec : None
fastparquet : None
gcsfs : None
matplotlib : 3.4.3
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : None
pyxlsb : None
s3fs : None
scipy : 1.7.1
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
numba : None
zstandard : None

[mirober]

The issue is with exceptions raised during the initialisation of e.g. PythonParser and CParserWrapper. It seems like some efforts have been made here to avoid file leaks but it's not done consistently, e.g. here we do check for exceptions:

https://github.com/pandas-dev/pandas/blob/main/pandas/io/parsers/c_parser_wrapper.py#L82-L86

But a few lines later there is another fallible call which isn't checked (the one which causes the issue in the repro):

https://github.com/pandas-dev/pandas/blob/main/pandas/io/parsers/c_parser_wrapper.py#L133-L134

It looks like this would be fairly easy to fix - everything after _open_handles in parser __init__ should be wrapped in a try/except which closes the open handles. Happy to raise a PR if this sounds right.

[twoertwein]

It looks like this would be fairly easy to fix - everything after _open_handles in parser __init__ should be wrapped in a try/except which closes the open handles. Happy to raise a PR if this sounds right.

Yes, feel free to open a PR (and also add a test that causes this error)!

[simonjayhawkins]

@twoertwein can you reproduce this? if so, can you check if a regression or long standing bug.

[twoertwein]

@twoertwein can you reproduce this? if so, can you check if a regression or long standing bug.

I can see that the file handle is leaked (but I cannot reproduce the unlink error since I don't have access to windows). Python disables ResourceWarnings by default, but running the example with python -W default you can see the warning: ResourceWarning: unclosed file <_io.TextIOWrapper name='file_leak_example.csv' mode='r' encoding='utf-8'>

It is very likely a bug introduced in 1.2 (centralized most of the IO in get_handle).

[twoertwein]

@mrob95 if you need a quick workaround, the following works on linux (probably also on windows):

import os

import pandas as pd

TEST_CONTENTS = """
col1,col2
a,b
1,2
"""
TEST_PATH = "file_leak_example.csv"
with open(TEST_PATH, "w+") as f:
    f.write(TEST_CONTENTS)
    f.seek(0)

    try:
        pd.read_csv(f, usecols=["col1", "col2", "col3"], engine="c")
    except:
        f.close()
        os.unlink(TEST_PATH)

[twoertwein]

@simonjayhawkins is that something that should be fixed before the very soon release of 1.4?

[mirober]

@mrob95 if you need a quick workaround, the following works on linux (probably also on windows):
...```

Thanks, I shipped this exact fix last week but only just got around to filing it :)

[simonjayhawkins]

@simonjayhawkins is that something that should be fixed before the very soon release of 1.4?

not necessarily for 1.4.0, but bug fixes and regression fixes can be backported to 1.4.x so if a regression should label as such so that the committer/reviewer can make a better judgment about backport.