From 743453a5541d3a6f19046459d22a7991ecb88223 Mon Sep 17 00:00:00 2001 From: Ev2geny Date: Wed, 5 Oct 2022 00:37:33 +0200 Subject: [PATCH] gh-58451: Add optional delete_on_close parameter to NamedTemporaryFile (GH-97015) --- Doc/library/tempfile.rst | 85 ++++++++++++--- Doc/whatsnew/3.12.rst | 5 + Lib/tempfile.py | 76 ++++++------- Lib/test/test_tempfile.py | 100 +++++++++++++++++- .../2020-09-28-04-56-04.bpo-14243.YECnxv.rst | 1 + 5 files changed, 216 insertions(+), 51 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-09-28-04-56-04.bpo-14243.YECnxv.rst diff --git a/Doc/library/tempfile.rst b/Doc/library/tempfile.rst index b7e604c1b70acb..b6d4f5dd05bbfc 100644 --- a/Doc/library/tempfile.rst +++ b/Doc/library/tempfile.rst @@ -75,20 +75,61 @@ The module defines the following user-callable items: Added *errors* parameter. -.. function:: NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, newline=None, suffix=None, prefix=None, dir=None, delete=True, *, errors=None) - - This function operates exactly as :func:`TemporaryFile` does, except that - the file is guaranteed to have a visible name in the file system (on - Unix, the directory entry is not unlinked). That name can be retrieved - from the :attr:`name` attribute of the returned - file-like object. Whether the name can be - used to open the file a second time, while the named temporary file is - still open, varies across platforms (it can be so used on Unix; it cannot - on Windows). If *delete* is true (the default), the file is - deleted as soon as it is closed. - The returned object is always a file-like object whose :attr:`!file` - attribute is the underlying true file object. This file-like object can - be used in a :keyword:`with` statement, just like a normal file. +.. function:: NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, newline=None, suffix=None, prefix=None, dir=None, delete=True, *, errors=None, delete_on_close=True) + + This function operates exactly as :func:`TemporaryFile` does, except the + following differences: + + * This function returns a file that is guaranteed to have a visible name in + the file system. + * To manage the named file, it extends the parameters of + :func:`TemporaryFile` with *delete* and *delete_on_close* parameters that + determine whether and how the named file should be automatically deleted. + + The returned object is always a :term:`file-like object` whose :attr:`!file` + attribute is the underlying true file object. This :term:`file-like object` + can be used in a :keyword:`with` statement, just like a normal file. The + name of the temporary file can be retrieved from the :attr:`name` attribute + of the returned file-like object. On Unix, unlike with the + :func:`TemporaryFile`, the directory entry does not get unlinked immediately + after the file creation. + + If *delete* is true (the default) and *delete_on_close* is true (the + default), the file is deleted as soon as it is closed. If *delete* is true + and *delete_on_close* is false, the file is deleted on context manager exit + only, or else when the :term:`file-like object` is finalized. Deletion is not + always guaranteed in this case (see :meth:`object.__del__`). If *delete* is + false, the value of *delete_on_close* is ignored. + + Therefore to use the name of the temporary file to reopen the file after + closing it, either make sure not to delete the file upon closure (set the + *delete* parameter to be false) or, in case the temporary file is created in + a :keyword:`with` statement, set the *delete_on_close* parameter to be false. + The latter approach is recommended as it provides assistance in automatic + cleaning of the temporary file upon the context manager exit. + + 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. + * On Windows, make sure that at least one of the following conditions are + fulfilled: + + * *delete* is false + * additional open shares delete access (e.g. by calling :func:`os.open` + with the flag ``O_TEMPORARY``) + * *delete* is true but *delete_on_close* is false. Note, that in this + case the additional opens that do not share delete access (e.g. + created via builtin :func:`open`) must be closed before exiting the + context manager, else the :func:`os.unlink` call on context manager + exit will fail with a :exc:`PermissionError`. + + On Windows, if *delete_on_close* is false, and the file is created in a + directory for which the user lacks delete access, then the :func:`os.unlink` + call on exit of the context manager will fail with a :exc:`PermissionError`. + This cannot happen when *delete_on_close* is true because delete access is + requested by the open, which fails immediately if the requested access is not + granted. On POSIX (only), a process that is terminated abruptly with SIGKILL cannot automatically delete any NamedTemporaryFiles it created. @@ -98,6 +139,9 @@ The module defines the following user-callable items: .. versionchanged:: 3.8 Added *errors* parameter. + .. versionchanged:: 3.12 + Added *delete_on_close* parameter. + .. class:: SpooledTemporaryFile(max_size=0, mode='w+b', buffering=-1, encoding=None, newline=None, suffix=None, prefix=None, dir=None, *, errors=None) @@ -346,6 +390,19 @@ Here are some examples of typical usage of the :mod:`tempfile` module:: >>> # file is now closed and removed + # create a temporary file using a context manager + # close the file, use the name to open the file again + >>> with tempfile.TemporaryFile(delete_on_close=False) as fp: + ... fp.write(b'Hello world!') + ... fp.close() + # the file is closed, but not removed + # open the file again by using its name + ... with open(fp.name) as f + ... f.read() + b'Hello world!' + >>> + # file is now removed + # create a temporary directory using the context manager >>> with tempfile.TemporaryDirectory() as tmpdirname: ... print('created temporary directory', tmpdirname) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 39ae2518bbdde1..052507a4873f81 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -148,6 +148,11 @@ unicodedata * The Unicode database has been updated to version 15.0.0. (Contributed by Benjamin Peterson in :gh:`96734`). +tempfile +-------- + +The :class:`tempfile.NamedTemporaryFile` function has a new optional parameter +*delete_on_close* (Contributed by Evgeny Zorin in :gh:`58451`.) Optimizations ============= diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 480c17232e9f09..bb18d60db0d919 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -418,42 +418,42 @@ class _TemporaryFileCloser: underlying file object, without adding a __del__ method to the temporary file.""" - file = None # Set here since __del__ checks it + cleanup_called = False close_called = False - def __init__(self, file, name, delete=True): + def __init__(self, file, name, delete=True, delete_on_close=True): self.file = file self.name = name self.delete = delete + self.delete_on_close = delete_on_close - # NT provides delete-on-close as a primitive, so we don't need - # the wrapper to do anything special. We still use it so that - # file.name is useful (i.e. not "(fdopen)") with NamedTemporaryFile. - if _os.name != 'nt': - # Cache the unlinker so we don't get spurious errors at - # shutdown when the module-level "os" is None'd out. Note - # that this must be referenced as self.unlink, because the - # name TemporaryFileWrapper may also get None'd out before - # __del__ is called. - - def close(self, unlink=_os.unlink): - if not self.close_called and self.file is not None: - self.close_called = True - try: + def cleanup(self, windows=(_os.name == 'nt'), unlink=_os.unlink): + if not self.cleanup_called: + self.cleanup_called = True + try: + if not self.close_called: + self.close_called = True self.file.close() - finally: - if self.delete: + finally: + # Windows provides delete-on-close as a primitive, in which + # case the file was deleted by self.file.close(). + if self.delete and not (windows and self.delete_on_close): + try: unlink(self.name) + except FileNotFoundError: + pass - # Need to ensure the file is deleted on __del__ - def __del__(self): - self.close() - - else: - def close(self): - if not self.close_called: - self.close_called = True + def close(self): + if not self.close_called: + self.close_called = True + try: self.file.close() + finally: + if self.delete and self.delete_on_close: + self.cleanup() + + def __del__(self): + self.cleanup() class _TemporaryFileWrapper: @@ -464,11 +464,11 @@ class _TemporaryFileWrapper: remove the file when it is no longer needed. """ - def __init__(self, file, name, delete=True): + def __init__(self, file, name, delete=True, delete_on_close=True): self.file = file self.name = name - self.delete = delete - self._closer = _TemporaryFileCloser(file, name, delete) + self._closer = _TemporaryFileCloser(file, name, delete, + delete_on_close) def __getattr__(self, name): # Attribute lookups are delegated to the underlying file @@ -499,7 +499,7 @@ def __enter__(self): # deleted when used in a with statement def __exit__(self, exc, value, tb): result = self.file.__exit__(exc, value, tb) - self.close() + self._closer.cleanup() return result def close(self): @@ -518,10 +518,10 @@ def __iter__(self): for line in self.file: yield line - def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, newline=None, suffix=None, prefix=None, - dir=None, delete=True, *, errors=None): + dir=None, delete=True, *, errors=None, + delete_on_close=True): """Create and return a temporary file. Arguments: 'prefix', 'suffix', 'dir' -- as for mkstemp. @@ -529,7 +529,10 @@ def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, 'buffering' -- the buffer size argument to io.open (default -1). 'encoding' -- the encoding argument to io.open (default None) 'newline' -- the newline argument to io.open (default None) - 'delete' -- whether the file is deleted on close (default True). + 'delete' -- whether the file is automatically deleted (default True). + 'delete_on_close' -- if 'delete', whether the file is deleted on close + (default True) or otherwise either on context manager exit + (if context manager was used) or on object finalization. . 'errors' -- the errors argument to io.open (default None) The file is created as mkstemp() would do it. @@ -548,7 +551,7 @@ def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, # Setting O_TEMPORARY in the flags causes the OS to delete # the file when it is closed. This is only supported by Windows. - if _os.name == 'nt' and delete: + if _os.name == 'nt' and delete and delete_on_close: flags |= _os.O_TEMPORARY if "b" not in mode: @@ -567,12 +570,13 @@ def opener(*args): raw = getattr(file, 'buffer', file) raw = getattr(raw, 'raw', raw) raw.name = name - return _TemporaryFileWrapper(file, name, delete) + return _TemporaryFileWrapper(file, name, delete, delete_on_close) except: file.close() raise except: - if name is not None and not (_os.name == 'nt' and delete): + if name is not None and not ( + _os.name == 'nt' and delete and delete_on_close): _os.unlink(name) raise diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index ccf7ea072de276..7c2c8de7a2e6fc 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -11,6 +11,7 @@ import stat import types import weakref +import gc from unittest import mock import unittest @@ -1013,6 +1014,102 @@ def use_closed(): pass self.assertRaises(ValueError, use_closed) + def test_context_man_not_del_on_close_if_delete_on_close_false(self): + # Issue gh-58451: tempfile.NamedTemporaryFile is not particulary useful + # on Windows + # A NamedTemporaryFile is NOT deleted when closed if + # delete_on_close=False, but is deleted on context manager exit + dir = tempfile.mkdtemp() + try: + with tempfile.NamedTemporaryFile(dir=dir, + delete=True, + delete_on_close=False) as f: + f.write(b'blat') + f_name = f.name + f.close() + with self.subTest(): + # Testing that file is not deleted on close + self.assertTrue(os.path.exists(f.name), + f"NamedTemporaryFile {f.name!r} is incorrectly " + f"deleted on closure when delete_on_close=False") + + with self.subTest(): + # Testing that file is deleted on context manager exit + self.assertFalse(os.path.exists(f.name), + f"NamedTemporaryFile {f.name!r} exists " + f"after context manager exit") + + finally: + os.rmdir(dir) + + def test_context_man_ok_to_delete_manually(self): + # In the case of delete=True, a NamedTemporaryFile can be manually + # deleted in a with-statement context without causing an error. + dir = tempfile.mkdtemp() + try: + with tempfile.NamedTemporaryFile(dir=dir, + delete=True, + delete_on_close=False) as f: + f.write(b'blat') + f.close() + os.unlink(f.name) + + finally: + os.rmdir(dir) + + def test_context_man_not_del_if_delete_false(self): + # A NamedTemporaryFile is not deleted if delete = False + dir = tempfile.mkdtemp() + f_name = "" + try: + # Test that delete_on_close=True has no effect if delete=False. + with tempfile.NamedTemporaryFile(dir=dir, delete=False, + delete_on_close=True) as f: + f.write(b'blat') + f_name = f.name + self.assertTrue(os.path.exists(f.name), + f"NamedTemporaryFile {f.name!r} exists after close") + finally: + os.unlink(f_name) + os.rmdir(dir) + + def test_del_by_finalizer(self): + # A NamedTemporaryFile is deleted when finalized in the case of + # delete=True, delete_on_close=False, and no with-statement is used. + def my_func(dir): + f = tempfile.NamedTemporaryFile(dir=dir, delete=True, + delete_on_close=False) + tmp_name = f.name + f.write(b'blat') + # Testing extreme case, where the file is not explicitly closed + # f.close() + return tmp_name + # Make sure that the garbage collector has finalized the file object. + gc.collect() + dir = tempfile.mkdtemp() + try: + tmp_name = my_func(dir) + self.assertFalse(os.path.exists(tmp_name), + f"NamedTemporaryFile {tmp_name!r} " + f"exists after finalizer ") + finally: + os.rmdir(dir) + + def test_correct_finalizer_work_if_already_deleted(self): + # There should be no error in the case of delete=True, + # delete_on_close=False, no with-statement is used, and the file is + # deleted manually. + def my_func(dir)->str: + f = tempfile.NamedTemporaryFile(dir=dir, delete=True, + delete_on_close=False) + tmp_name = f.name + f.write(b'blat') + f.close() + os.unlink(tmp_name) + return tmp_name + # Make sure that the garbage collector has finalized the file object. + gc.collect() + def test_bad_mode(self): dir = tempfile.mkdtemp() self.addCleanup(os_helper.rmtree, dir) @@ -1081,7 +1178,8 @@ def test_iobase_interface(self): missing_attrs = iobase_attrs - spooledtempfile_attrs self.assertFalse( missing_attrs, - 'SpooledTemporaryFile missing attributes from IOBase/BufferedIOBase/TextIOBase' + 'SpooledTemporaryFile missing attributes from ' + 'IOBase/BufferedIOBase/TextIOBase' ) def test_del_on_close(self): diff --git a/Misc/NEWS.d/next/Library/2020-09-28-04-56-04.bpo-14243.YECnxv.rst b/Misc/NEWS.d/next/Library/2020-09-28-04-56-04.bpo-14243.YECnxv.rst new file mode 100644 index 00000000000000..267535452ef146 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-09-28-04-56-04.bpo-14243.YECnxv.rst @@ -0,0 +1 @@ +The :class:`tempfile.NamedTemporaryFile` function has a new optional parameter *delete_on_close*