python / cpython

The Python programming language
https://www.python.org
Other
63.69k stars 30.51k forks source link

NamedTemporaryFile doesn't issue a ResourceWarning when left unclosed on POSIX #126639

Open graingert opened 2 weeks ago

graingert commented 2 weeks ago

Bug report

Bug description:

import sys
import tempfile

def main():
    tempfile.NamedTemporaryFile()

if __name__ == "__main__":
    sys.exit(main())

when run with python -Werror demo.py nothing happens

if you open a file normally you get a ResourceWarning:

import sys
import tempfile

def main():
    open("example", "w")

if __name__ == "__main__":
    sys.exit(main())
 graingert@conscientious  ~/projects/temp_file_resource_warnings  python -Werror demo2.py
Exception ignored in: <_io.FileIO name='example' mode='wb' closefd=True>
Traceback (most recent call last):
  File "/home/graingert/projects/temp_file_resource_warnings/demo2.py", line 6, in main
    open("example", "w")
ResourceWarning: unclosed file <_io.TextIOWrapper name='example' mode='w' encoding='UTF-8'>

it appears that you do get a ResourceWarning on windows, but I've only seen it in CI I havn't reproduced locally yet

CPython versions tested on:

3.12

Operating systems tested on:

Linux

Linked PRs

graingert commented 2 weeks ago

I think this __del__ needs to issue the resource warning:

https://github.com/python/cpython/blob/450db61a78989c5a1f1106be01e071798c783cf9/Lib/tempfile.py#L471-L472

picnixz commented 2 weeks ago

I think this del needs to issue the resource warning:

On POSIX, the docs say

Opening the temporary file again by its name while it is still open works as follows: On POSIX the file can always be opened again.

So technically, no warning should be issued since you can leave it opened (I think?).

graingert commented 2 weeks ago

I don't see how that's related? The resource delete on __del__ is a last resort, and it needs to issue a ResourceWarning so that code that fails to finalise explicitly can be detected

picnixz commented 2 weeks ago

Ah sorry, I misunderstood the "open again" in the sense that you can leave the descriptor hanging. My bad. However, non-named temporary files say:

It will be destroyed as soon as it is closed (including an implicit close when the object is garbage collected).

Should implicit close emit a warning in this case? Actually, why is there a warning on Windows?

graingert commented 2 weeks ago

Should implicit close emit a warning in this case?

Yes, all implicit closing of resources on GC should issue a ResourceWarning

graingert commented 2 weeks ago

I suspect the way to go here is to replace _TemporaryFileCloser with a weakref.finalize

tomasr8 commented 2 weeks ago

I suspect the way to go here is to replace _TemporaryFileCloser with a weakref.finalize

Looks like it, TemporaryDirectory is doing something similar:

https://github.com/python/cpython/blob/450db61a78989c5a1f1106be01e071798c783cf9/Lib/tempfile.py#L885-L888