gh-58451: Add optional delete_on_close parameter to NamedTemporaryFile

Doc/library/tempfile.rst

@@ -75,20 +75,53 @@ 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:
+
+   * The file is guaranteed to have a visible name in the file system (on Unix,
+     the directory entry is not unlinked).
+
+   * There is more granularity in the deletion behaviour of the file (see
+     ``delete_on_close`` below)
+
+   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.
+
+   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, if no context manager was used, then the file is deleted only when the
+   :term:`file-like object` is finalized and hence deletion is not always
+   guaranteed in this case (see :meth:`object.__del__`). If *delete* is false,
+   the value of *delete_on_close* is ignored.
+
+   While the named temporary file is open, the file can always be opened again
+   on POSIX. On Windows, it can be opened again if *delete* is false, or if
+   *delete_on_close* is false, or if the additional open shares delete access
+   (e.g. by calling :func:`os.open` with the flag ``O_TEMPORARY``).  On
+   Windows, if *delete* is true and *delete_on_close* is false, additional
+   opens that do not share delete access (e.g. 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`.
+
+   To use the name of the temporary file to open the closed file second time,
+   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.
+
+   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 +131,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 +382,22 @@ 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, note the name,
+    # 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_name = fp.name
+    ...     fp.close()
+    # the file is closed, but not removed
+    # open the file again by using its name
+    ...     f = open(fp_name)
+    ...     f.seek(0)
+    ...     f.read()
+    b'Hello world!'
+    ...     f.close()
+    >>>
+    # file is now removed
+
     # create a temporary directory using the context manager
     >>> with tempfile.TemporaryDirectory() as tmpdirname:
     ...     print('created temporary directory', tmpdirname)

Doc/whatsnew/3.12.rst

@@ -142,6 +142,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
 =============

Lib/tempfile.py

@@ -421,10 +421,11 @@ class _TemporaryFileCloser:
     file = None  # Set here since __del__ checks it
     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
@@ -442,19 +443,22 @@ def close(self, unlink=_os.unlink):
                 try:
                     self.file.close()
                 finally:
-                    if self.delete:
+                    if self.delete and self.delete_on_close:
                         unlink(self.name)
 
-        # 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
                 self.file.close()
 
+    def __del__(self):
+        self.close()
+        # Need to ensure the file is deleted on __del__ if delete = True and
+        # file still exists
+        if self.delete and _os.path.exists(self.name):
+            _os.unlink(self.name)
 
 class _TemporaryFileWrapper:
     """Temporary file wrapper
@@ -464,11 +468,13 @@ 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.delete_on_close = delete_on_close
+        self._closer = _TemporaryFileCloser(file, name, delete,
+                                            delete_on_close)
 
     def __getattr__(self, name):
         # Attribute lookups are delegated to the underlying file
@@ -500,6 +506,15 @@ def __enter__(self):
     def __exit__(self, exc, value, tb):
         result = self.file.__exit__(exc, value, tb)
         self.close()
+        # If the file is to be deleted, but not on close, delete it now.
+        if self.delete and not self.delete_on_close:
+            try:
+                _os.unlink(self.name)
+            # It is okay to ignore FileNotFoundError. The file may have
+            # been deleted already.
+            except FileNotFoundError:
+                pass
+
         return result
 
     def close(self):
@@ -521,15 +536,18 @@ def __iter__(self):
 
 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.
     'mode' -- the mode argument to io.open (default "w+b").
     '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
+       or on context exit (default True).
     'errors' -- the errors argument to io.open (default None)
     The file is created as mkstemp() would do it.
 
@@ -548,7 +566,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,7 +585,7 @@ 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

Lib/test/test_tempfile.py

@@ -11,6 +11,7 @@
 import stat
 import types
 import weakref
+import gc
 from unittest import mock
 
 import unittest
@@ -979,6 +980,91 @@ def test_del_on_close(self):
         finally:
             os.rmdir(dir)
 
+    def test_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 content 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),
+                            "NamedTemporaryFile %s is incorrectly deleted\
+                             on closure when delete_on_close= False" % f_name)
+
+            with self.subTest():
+                # Testing that file is deleted on content manager exit
+                self.assertFalse(os.path.exists(f.name),
+                                 "NamedTemporaryFile %s exists\
+                                  after content manager exit" % f.name)
+
+        finally:
+            os.rmdir(dir)
+
+    def test_ok_to_delete_manually(self):
+        # A NamedTemporaryFile can be deleted by a user before content manager
+        # comes to it. This will not generate 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_not_del_if_delete_false(self):
+        # A NamedTemporaryFile is not deleted if delete = False
+        dir = tempfile.mkdtemp()
+        f_name = ""
+        try:
+            # setting delete_on_close = True to test, that this does not have
+            # an 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),
+                        "NamedTemporaryFile %s exists after close" % f.name)
+        finally:
+            os.unlink(f_name)
+            os.rmdir(dir)
+
+    def test_del_by_finalizer_if_no_with(self):
+        # A NamedTemporaryFile is deleted by fanalizer in case delete = True
+        # delete_on_close = False and no context manager 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')
+            f.close()
+            with self.subTest():
+                self.assertTrue(os.path.exists(tmp_name),
+                            "NamedTemporaryFile %s missing after close"\
+                                % tmp_name)
+            return tmp_name
+        # Making sure that Garbage Collector has finalyzed the file object
+        gc.collect()
+        dir = tempfile.mkdtemp()
+        try:
+            tmp_name = my_func(dir)
+            with self.subTest():
+                self.assertFalse(os.path.exists(tmp_name),
+                            "NamedTemporaryFile %s exists after finalizer "\
+                                % tmp_name)
+        finally:
+            os.rmdir(dir)
+
     def test_dis_del_on_close(self):
         # Tests that delete-on-close can be disabled
         dir = tempfile.mkdtemp()

Misc/NEWS.d/next/Library/2020-09-28-04-56-04.bpo-14243.YECnxv.rst

@@ -0,0 +1 @@
+The :class:`tempfile.NamedTemporaryFile` has a new optional parameter *delete_on_close*