Issue #10188 (partial resolution): tidy up some behaviour in the new tempfile.TemporaryDirectory context manager

4 files changed, 79 insertions, 14 deletions

diff --git a/Lib/tempfile.py b/Lib/tempfile.py
index 7e03784..b28d91f 100644
--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -29,6 +29,8 @@ __all__ = [
 
 # Imports.
 
+import warnings as _warnings
+import sys as _sys
 import io as _io
 import os as _os
 import errno as _errno
@@ -617,24 +619,40 @@ class TemporaryDirectory(object):
     """
 
     def __init__(self, suffix="", prefix=template, dir=None):
-        # cleanup() needs this and is called even when mkdtemp fails
-        self._closed = True
-        self.name = mkdtemp(suffix, prefix, dir)
         self._closed = False
+        self.name = None # Handle mkdtemp throwing an exception
+        self.name = mkdtemp(suffix, prefix, dir)
+
+    def __repr__(self):
+        return "<{} {!r}>".format(self.__class__.__name__, self.name)
 
     def __enter__(self):
         return self.name
 
-    def cleanup(self):
-        if not self._closed:
-            self._rmtree(self.name)
+    def cleanup(self, _warn=False):
+        if self.name and not self._closed:
+            try:
+                self._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):
+                    raise
+                print("ERROR: {!r} while cleaning up {!r}".format(ex, self,),
+                      file=_sys.stderr)
+                return
             self._closed = True
+            if _warn:
+                self._warn("Implicitly cleaning up {!r}".format(self),
+                           ResourceWarning)
 
     def __exit__(self, exc, value, tb):
         self.cleanup()
 
-    __del__ = cleanup
-
+    def __del__(self):
+        # 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
@@ -646,6 +664,7 @@ class TemporaryDirectory(object):
     _remove = staticmethod(_os.remove)
     _rmdir = staticmethod(_os.rmdir)
     _os_error = _os.error
+    _warn = _warnings.warn
 
     def _rmtree(self, path):
         # Essentially a stripped down version of shutil.rmtree.  We can't
diff --git a/Lib/test/support.py b/Lib/test/support.py
index 2fd019a..27efd37 100644
--- a/Lib/test/support.py
+++ b/Lib/test/support.py
@@ -874,6 +874,9 @@ def captured_output(stream_name):
 def captured_stdout():
     return captured_output("stdout")
 
+def captured_stderr():
+    return captured_output("stderr")
+
 def captured_stdin():
     return captured_output("stdin")
 
diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py
index 802db1f..eebf78f 100644
--- a/Lib/test/test_tempfile.py
+++ b/Lib/test/test_tempfile.py
@@ -925,6 +925,13 @@ class test_TemporaryDirectory(TC):
             f.write(b"Hello world!")
         return tmp
 
+    def test_mkdtemp_failure(self):
+        # Check no additional exception if mkdtemp fails
+        # Previously would raise AttributeError instead
+        # (noted as part of Issue #10888)
+        #with self.assertRaises(os.error):
+        tempfile.TemporaryDirectory(prefix="[]<>?*!:")
+
     def test_explicit_cleanup(self):
         # A TemporaryDirectory is deleted when cleaned up
         dir = tempfile.mkdtemp()
@@ -955,20 +962,50 @@ class test_TemporaryDirectory(TC):
     def test_del_on_shutdown(self):
         # A TemporaryDirectory may be cleaned up during shutdown
         # Make sure it works with the relevant modules nulled out
-        dir = tempfile.mkdtemp()
-        try:
+        with self.do_create() as dir:
             d = self.do_create(dir=dir)
             # Mimic the nulling out of modules that
             # occurs during system shutdown
             modules = [os, os.path]
             if has_stat:
                 modules.append(stat)
-            with NulledModules(*modules):
-                d.cleanup()
+            # Currently broken, so suppress the warning
+            # that is otherwise emitted on stdout
+            with support.captured_stderr() as err:
+                with NulledModules(*modules):
+                    d.cleanup()
+            # Currently broken, so stop spurious exception by
+            # indicating the object has already been closed
+            d._closed = True
+            # And this assert will fail, as expected by the
+            # unittest decorator...
             self.assertFalse(os.path.exists(d.name),
                         "TemporaryDirectory %s exists after cleanup" % d.name)
-        finally:
-            os.rmdir(dir)
+
+    def test_warnings_on_cleanup(self):
+        # Two kinds of warning on shutdown
+        #   Issue 10888: may write to stderr if modules are nulled out
+        #   ResourceWarning will be triggered by __del__
+        with self.do_create() as dir:
+            d = self.do_create(dir=dir)
+
+            #Check for the Issue 10888 message
+            modules = [os, os.path]
+            if has_stat:
+                modules.append(stat)
+            with support.captured_stderr() as err:
+                with NulledModules(*modules):
+                    d.cleanup()
+            message = err.getvalue()
+            self.assertIn("while cleaning up",  message)
+            self.assertIn(d.name,  message)
+
+            # Check for the resource warning
+            with support.check_warnings(('Implicitly', ResourceWarning), quiet=False):
+                warnings.filterwarnings("always", category=ResourceWarning)
+                d.__del__()
+            self.assertFalse(os.path.exists(d.name),
+                        "TemporaryDirectory %s exists after __del__" % d.name)
 
     def test_multiple_close(self):
         # Can be cleaned-up many times without error
diff --git a/Misc/NEWS b/Misc/NEWS
index 080c86e..af34fe5 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -11,6 +11,12 @@ What's New in Python 3.2 Beta 2?
 Library
 -------
 
+* Issue #10188 (partial resolution): tempfile.TemporaryDirectory emits
+  a warning on sys.stderr rather than throwing a misleading exception
+  if cleanup fails due to nulling out of modules during shutdown.
+  Also avoids an AttributeError when mkdtemp call fails and issues
+  a ResourceWarning on implicit cleanup via __del__.
+
 * Issue #10107: Warn about unsaved files in IDLE on OSX.