mirror of https://github.com/python/cpython
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.
This commit is contained in:
Serhiy Storchaka
commit
a28632be56
|
|
@ -29,11 +29,12 @@ __all__ = [
|
|||
|
||||
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 @@ class _TemporaryFileCloser:
|
|||
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 @@ 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):
|
||||
|
|
@ -657,10 +659,23 @@ class TemporaryDirectory(object):
|
|||
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 @@ class TemporaryDirectory(object):
|
|||
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
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ import contextlib
|
|||
import weakref
|
||||
|
||||
import unittest
|
||||
from test import support
|
||||
from test import support, script_helper
|
||||
|
||||
|
||||
if hasattr(os, 'stat'):
|
||||
|
|
@ -1088,7 +1088,8 @@ class TestTemporaryDirectory(BaseTestCase):
|
|||
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 @@ class TestTemporaryDirectory(BaseTestCase):
|
|||
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 @@ class TestTemporaryDirectory(BaseTestCase):
|
|||
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
|
||||
|
|
|
|||
|
|
@ -48,6 +48,10 @@ Core and Builtins
|
|||
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
|
||||
|
|
|
|||
Loading…
Reference in New Issue