Issue #19077: tempfile.TemporaryDirectory cleanup is now most likely

successful when called during nulling out of modules during shutdown.
Misleading exception no longer raised when resource warning is emitted
during shutdown.

1 files changed, 46 insertions, 49 deletions

diff --git a/Lib/tempfile.py b/Lib/tempfile.py
index b4b5c88..6bc842f 100644
--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -27,11 +27,12 @@ __all__ = [
 
 # Imports.
 
+import atexit as _atexit
 import functools as _functools
 import warnings as _warnings
-import sys as _sys
 import io as _io
 import os as _os
+import shutil as _shutil
 import errno as _errno
 from random import Random as _Random
 
@@ -355,10 +356,13 @@ class _TemporaryFileCloser:
     underlying file object, without adding a __del__ method to the
     temporary file."""
 
+    # Set here since __del__ checks it
+    file = None
+    close_called = False
+
     def __init__(self, file, name, delete=True):
         self.file = file
         self.name = name
-        self.close_called = False
         self.delete = delete
 
     # NT provides delete-on-close as a primitive, so we don't need
@@ -370,14 +374,13 @@ class _TemporaryFileCloser:
         # that this must be referenced as self.unlink, because the
         # name TemporaryFileWrapper may also get None'd out before
         # __del__ is called.
-        unlink = _os.unlink
 
-        def close(self):
-            if not self.close_called:
+        def close(self, unlink=_os.unlink):
+            if not self.close_called and self.file is not None:
                 self.close_called = True
                 self.file.close()
                 if self.delete:
-                    self.unlink(self.name)
+                    unlink(self.name)
 
         # Need to ensure the file is deleted on __del__
         def __del__(self):
@@ -677,9 +680,11 @@ class TemporaryDirectory(object):
     in it are removed.
     """
 
+    # Handle mkdtemp raising an exception
+    name = None
+    _closed = False
+
     def __init__(self, suffix="", prefix=template, dir=None):
-        self._closed = False
-        self.name = None # Handle mkdtemp raising an exception
         self.name = mkdtemp(suffix, prefix, dir)
 
     def __repr__(self):
@@ -688,23 +693,24 @@ class TemporaryDirectory(object):
     def __enter__(self):
         return self.name
 
-    def cleanup(self, _warn=False):
+    def cleanup(self, _warn=False, _warnings=_warnings):
         if self.name and not self._closed:
             try:
-                self._rmtree(self.name)
+                _shutil.rmtree(self.name)
             except (TypeError, AttributeError) as ex:
-                # Issue #10188: Emit a warning on stderr
-                # if the directory could not be cleaned
-                # up due to missing globals
-                if "None" not in str(ex):
+                if "None" not in '%s' % (ex,):
                     raise
-                print("ERROR: {!r} while cleaning up {!r}".format(ex, self,),
-                      file=_sys.stderr)
-                return
+                self._rmtree(self.name)
             self._closed = True
-            if _warn:
-                self._warn("Implicitly cleaning up {!r}".format(self),
-                           ResourceWarning)
+            if _warn and _warnings.warn:
+                try:
+                    _warnings.warn("Implicitly cleaning up {!r}".format(self),
+                                   ResourceWarning)
+                except:
+                    if _is_running:
+                        raise
+                    # Don't raise an exception if modules needed for emitting
+                    # a warning are already cleaned in shutdown process.
 
     def __exit__(self, exc, value, tb):
         self.cleanup()
@@ -713,36 +719,27 @@ class TemporaryDirectory(object):
         # Issue a ResourceWarning if implicit cleanup needed
         self.cleanup(_warn=True)
 
-    # XXX (ncoghlan): The following code attempts to make
-    # this class tolerant of the module nulling out process
-    # that happens during CPython interpreter shutdown
-    # Alas, it doesn't actually manage it. See issue #10188
-    _listdir = staticmethod(_os.listdir)
-    _path_join = staticmethod(_os.path.join)
-    _isdir = staticmethod(_os.path.isdir)
-    _islink = staticmethod(_os.path.islink)
-    _remove = staticmethod(_os.remove)
-    _rmdir = staticmethod(_os.rmdir)
-    _os_error = OSError
-    _warn = _warnings.warn
-
-    def _rmtree(self, path):
+    def _rmtree(self, path, _OSError=OSError, _sep=_os.path.sep,
+                _listdir=_os.listdir, _remove=_os.remove, _rmdir=_os.rmdir):
         # Essentially a stripped down version of shutil.rmtree.  We can't
         # use globals because they may be None'ed out at shutdown.
-        for name in self._listdir(path):
-            fullname = self._path_join(path, name)
-            try:
-                isdir = self._isdir(fullname) and not self._islink(fullname)
-            except self._os_error:
-                isdir = False
-            if isdir:
-                self._rmtree(fullname)
-            else:
-                try:
-                    self._remove(fullname)
-                except self._os_error:
-                    pass
+        if not isinstance(path, str):
+            _sep = _sep.encode()
         try:
-            self._rmdir(path)
-        except self._os_error:
+            for name in _listdir(path):
+                fullname = path + _sep + name
+                try:
+                    _remove(fullname)
+                except _OSError:
+                    self._rmtree(fullname)
+            _rmdir(path)
+        except _OSError:
             pass
+
+_is_running = True
+
+def _on_shutdown():
+    global _is_running
+    _is_running = False
+
+_atexit.register(_on_shutdown)