search

cylc / cylc-flow

Cylc: a workflow engine for cycling systems.
https://cylc.github.io
GNU General Public License v3.0
335 stars 94 forks source link

warnings: log_vc_info - unclosed file #5989

Open oliver-sanders opened 9 months ago

oliver-sanders commented 9 months ago

Address a Python warning:

cylc-flow/cylc/flow/install_plugins/log_vc_info.py:185: ResourceWarning: unclosed file <_io.TextIOWrapper name=5 encoding='UTF-8'>
  continue
ResourceWarning: Enable tracemalloc to get the object allocation traceback
cylc-flow/cylc/flow/install_plugins/log_vc_info.py:185: ResourceWarning: unclosed file <_io.TextIOWrapper name=7 encoding='UTF-8'>
  continue
ResourceWarning: Enable tracemalloc to get the object allocation traceback
cylc-flow/cylc/flow/install_plugins/log_vc_info.py:185: ResourceWarning: unclosed file <_io.TextIOWrapper name=5 encoding='UTF-8'>
  continue
ResourceWarning: Enable tracemalloc to get the object allocation traceback
cylc-flow/cylc/flow/install_plugins/log_vc_info.py:185: ResourceWarning: unclosed file <_io.TextIOWrapper name=7 encoding='UTF-8'>
  continue

To see this warning, run the tests or try adding:

import warnings
warnings.filterwarnings('once')
MetRonnie commented 9 months ago

I've narrowed this down to pipe_poller (by commenting out the first branch here and seeing the warnings disappear): https://github.com/cylc/cylc-flow/blob/8b2feaea91ac9fa1da80d87274781bb5a25ad30b/cylc/flow/install_plugins/log_vc_info.py#L257-L260

https://github.com/cylc/cylc-flow/blob/8b2feaea91ac9fa1da80d87274781bb5a25ad30b/cylc/flow/pipe_poller.py#L29-L30

oliver-sanders commented 9 months ago

Good spot, this may have the same underlying cause as https://github.com/cylc/cylc-flow/issues/5990

oliver-sanders commented 8 months ago

Doesn't make sense to me, the pipe_poller does not open any files, it just reads from already open files. It wouldn't make sense for the pipe_poller to close the files, it might not even be provided with all of them.

Is proc.communicate closing proc.out and proc.err? If so, then we just need to add that into the pipe_poller branch. Or possible proc.wait if that also closes the files?

MetRonnie commented 8 months ago

Is proc.communicate closing proc.out and proc.err?

Yes looks like it. When it reaches the end of the file it closes it - https://github.com/python/cpython/blob/ecd9946c9d1df7cd1aa15d08d72bbcc0899d272d/Lib/subprocess.py#L1750-L1754

oliver-sanders commented 8 months ago

Ok, then we can bung a proc.communicate after or possibly even inside the proc_poller bit to resolve this.