Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP/CI: Debug ResourceWarnings unclosed io.BufferedRandom #44634

Closed
wants to merge 24 commits into from

Conversation

@mroeschke mroeschke marked this pull request as draft November 27, 2021 00:08
@mroeschke
Copy link
Member Author

Note using psutil in tm.assert_produces_warning like

                warning_data = [
                    actual_warning.category.__name__,
                    actual_warning.message,
                    actual_warning.filename,
                    actual_warning.lineno,
                ]
    
                if actual_warning.category == ResourceWarning:
                    import psutil
    
                    proc = psutil.Process()
                    flist = proc.open_files()
                    warning_data.append(flist)
    
                extra_warnings.append(tuple(warning_data))

Doesn't seem to uncover files: https://github.com/pandas-dev/pandas/runs/4348313550?check_suite_focus=true

>           raise AssertionError(f"Caused unexpected warning(s): {repr(extra_warnings)}")
E           AssertionError: Caused unexpected warning(s): [('ResourceWarning', ResourceWarning('unclosed file <_io.BufferedRandom name=16>'), '/usr/share/miniconda/envs/pandas-dev/lib/python3.9/site-packages/jinja2/runtime.py', 365, [])]

@jbrockmendel
Copy link
Member

i guess BufferedRandom isn't a "real" file object? (which would also explain why td.check_file_leaks isn't catching it).
i guess could try to see if any of our dependencies use these objects?

the docs mention that BufferedRandom is threadsafe, so maybe the flakiness involves gc timing?

@jbrockmendel
Copy link
Member

could try passing close_fds=True to subprocess.Popen in conftest

@mroeschke
Copy link
Member Author

Looks like close_fds=True is the default for Popen on all platforms since 3.7

https://docs.python.org/3/library/subprocess.html#popen-constructor

@mroeschke
Copy link
Member Author

Looking at cpython source, one hint is that this object was opened in + mode.

https://github.com/python/cpython/blob/f4c03484da59049eb62a9bf7777b963e2267d187/Lib/_pyio.py#L259

Not pointing fingers necessarily, but tm.ensure_clean opens a file handle in + mode. We've been using that function for a while now though.

@jbrockmendel
Copy link
Member

@pganssle we're trying to track down ResourceWarnings caused by unclosed io.BufferedRandom objects. AFAICT these are not being seen by psutil.open_files. Looking for a way to check "give me a list of all unclosed BufferedRandom objects". Suggestions on either how to do this or where to open an issue about how to do this?

@johnzangwill
Copy link
Contributor

johnzangwill commented Nov 30, 2021

CI seems to fail randomly about 50% of the time, sometimes with this error. My last PR push failed in test_xlswriter.py (See log). I just re-triggered CI to get my green tick!

io.BufferedRandom unclosed file log: unclosed file.log

@mroeschke
Copy link
Member Author

mroeschke commented Nov 30, 2021

Another suspect from ipython, a file opened in r+ mode:

2021-11-29T21:59:39.5999240Z ...[popenfile(path='/home/runner/.ipython/profile_default/history.sqlite', fd=8, position=0, mode='r+', flags=688130), popenfile(path='/home/runner/.ipython/profile_default/history.sqlite', fd=9, position=0, mode='r+', flags=688130)]

https://github.com/ipython/ipython/blob/ef1d8add5d975b88bfcbccb6fecf146b29a15154/IPython/core/history.py
https://github.com/ipython/ipython/blob/66843b1ea573772309a6b29a69b2afd4641ae0d1/IPython/core/historyapp.py#L56

@mroeschke
Copy link
Member Author

Looks like you had addressed leaking iPython files in the past @jbrockmendel: #35836

@mroeschke
Copy link
Member Author

Assuming #44700 addressed the io.BufferedRandom, closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants