[Python-checkins] cpython (3.3): Issue #19077: tempfile.TemporaryDirectory cleanup is now most likely
serhiy.storchaka
python-checkins at python.org
Mon Jan 27 10:33:41 CET 2014
http://hg.python.org/cpython/rev/50aa9e3ab9a4
changeset: 88771:50aa9e3ab9a4
branch: 3.3
parent: 88765:791b69f9f96d
user: Serhiy Storchaka <storchaka at gmail.com>
date: Mon Jan 27 11:18:27 2014 +0200
summary:
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.
files:
Lib/tempfile.py | 97 +++++++++++++-------------
Lib/test/test_tempfile.py | 84 ++++++++++-------------
Misc/NEWS | 5 +
3 files changed, 89 insertions(+), 97 deletions(-)
diff --git a/Lib/tempfile.py b/Lib/tempfile.py
--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -27,11 +27,12 @@
# 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 @@
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 @@
# 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 @@
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 @@
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:
+ _shutil.rmtree(self.name)
+ except (TypeError, AttributeError) as ex:
+ if "None" not in '%s' % (ex,):
+ raise
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)
+ 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 @@
# 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:
+ if not isinstance(path, str):
+ _sep = _sep.encode()
+ try:
+ for name in _listdir(path):
+ fullname = path + _sep + name
try:
- self._remove(fullname)
- except self._os_error:
- pass
- try:
- self._rmdir(path)
- except self._os_error:
+ _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)
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'):
@@ -1073,7 +1073,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
@@ -1105,7 +1106,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"))
@@ -1135,60 +1136,49 @@
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 ('os', 'shutil', 'sys', 'tempfile', 'warnings'):
+ code = """if True:
+ 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)
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
@@ -50,6 +50,11 @@
Library
-------
+- 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.
+
- Issue #20367: Fix behavior of concurrent.futures.as_completed() for
duplicate arguments. Patch by Glenn Langford.
--
Repository URL: http://hg.python.org/cpython
More information about the Python-checkins
mailing list