http://hg.python.org/cpython/rev/d792eb3afa58 changeset: 88772:d792eb3afa58 parent: 88770:0eec47f9f8a1 parent: 88771:50aa9e3ab9a4 user: Serhiy Storchaka <storchaka@gmail.com> date: Mon Jan 27 11:21:54 2014 +0200 summary: Issue #19077: tempfile.TemporaryDirectory cleanup no longer fails when called during shutdown. Emitting resource warning in __del__ no longer fails. Original patch by Antoine Pitrou. files: Lib/tempfile.py | 90 ++++++++------------------ Lib/test/test_tempfile.py | 86 +++++++++++-------------- Misc/NEWS | 4 + 3 files changed, 72 insertions(+), 108 deletions(-) diff --git a/Lib/tempfile.py b/Lib/tempfile.py --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -29,11 +29,12 @@ 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 +import weakref as _weakref try: import _thread @@ -335,10 +336,12 @@ underlying file object, without adding a __del__ method to the temporary file.""" + file = None # Set here since __del__ checks it + 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 @@ -350,14 +353,13 @@ # 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): @@ -657,10 +659,23 @@ in it are removed. """ + # Handle mkdtemp raising an exception + name = None + _finalizer = 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) + self._finalizer = _weakref.finalize( + self, self._cleanup, self.name, + warn_message="Implicitly cleaning up {!r}".format(self)) + + @classmethod + def _cleanup(cls, name, warn_message=None): + _shutil.rmtree(name) + if warn_message is not None: + _warnings.warn(warn_message, ResourceWarning) + def __repr__(self): return "<{} {!r}>".format(self.__class__.__name__, self.name) @@ -668,60 +683,13 @@ def __enter__(self): return 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() - def __del__(self): - # Issue a ResourceWarning if implicit cleanup needed - self.cleanup(_warn=True) + def cleanup(self): + if self._finalizer is not None: + self._finalizer.detach() + if self.name is not None and not self._closed: + _shutil.rmtree(self.name) + self._closed = 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) - _warn = _warnings.warn - - def _rmtree(self, path): - # 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 OSError: - isdir = False - if isdir: - self._rmtree(fullname) - else: - try: - self._remove(fullname) - except OSError: - pass - try: - self._rmdir(path) - except OSError: - pass diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -11,7 +11,7 @@ import weakref import unittest -from test import support +from test import support, script_helper if hasattr(os, 'stat'): @@ -1088,7 +1088,8 @@ self.nameCheck(tmp.name, dir, pre, suf) # Create a subdirectory and some files if recurse: - self.do_create(tmp.name, pre, suf, recurse-1) + d1 = self.do_create(tmp.name, pre, suf, recurse-1) + d1.name = None with open(os.path.join(tmp.name, "test.txt"), "wb") as f: f.write(b"Hello world!") return tmp @@ -1120,7 +1121,7 @@ def test_cleanup_with_symlink_to_a_directory(self): # cleanup() should not follow symlinks to directories (issue #12464) d1 = self.do_create() - d2 = self.do_create() + d2 = self.do_create(recurse=0) # Symlink d1/foo -> d2 os.symlink(d2.name, os.path.join(d1.name, "foo")) @@ -1150,60 +1151,51 @@ finally: os.rmdir(dir) - @unittest.expectedFailure # See issue #10188 def test_del_on_shutdown(self): # A TemporaryDirectory may be cleaned up during shutdown - # Make sure it works with the relevant modules nulled out 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) - # 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) + for mod in ('builtins', 'os', 'shutil', 'sys', 'tempfile', 'warnings'): + code = """if True: + import builtins + import os + import shutil + import sys + import tempfile + import warnings + + tmp = tempfile.TemporaryDirectory(dir={dir!r}) + sys.stdout.buffer.write(tmp.name.encode()) + + tmp2 = os.path.join(tmp.name, 'test_dir') + os.mkdir(tmp2) + with open(os.path.join(tmp2, "test.txt"), "w") as f: + f.write("Hello world!") + + {mod}.tmp = tmp + + warnings.filterwarnings("always", category=ResourceWarning) + """.format(dir=dir, mod=mod) + rc, out, err = script_helper.assert_python_ok("-c", code) + tmp_name = out.decode().strip() + self.assertFalse(os.path.exists(tmp_name), + "TemporaryDirectory %s exists after cleanup" % tmp_name) + err = err.decode('utf-8', 'backslashreplace') + self.assertNotIn("Exception ", err) + self.assertIn("ResourceWarning: Implicitly cleaning up", err) 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__ + # ResourceWarning will be triggered by __del__ with self.do_create() as dir: - if os.sep != '\\': - # Embed a backslash in order to make sure string escaping - # in the displayed error message is dealt with correctly - suffix = '\\check_backslash_handling' - else: - suffix = '' - d = self.do_create(dir=dir, suf=suffix) - - #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().replace('\\\\', '\\') - self.assertIn("while cleaning up", message) - self.assertIn(d.name, message) + d = self.do_create(dir=dir, recurse=3) + name = d.name # 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) + del d + support.gc_collect() + self.assertFalse(os.path.exists(name), + "TemporaryDirectory %s exists after __del__" % name) def test_multiple_close(self): # Can be cleaned-up many times without error diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -48,6 +48,10 @@ Library ------- +- Issue #19077: tempfile.TemporaryDirectory cleanup no longer fails when + called during shutdown. Emitting resource warning in __del__ no longer fails. + Original patch by Antoine Pitrou. + - Issue #20394: Silence Coverity warning in audioop module. - Issue #20367: Fix behavior of concurrent.futures.as_completed() for -- Repository URL: http://hg.python.org/cpython