Skip to content

Commit

Permalink
tempfile.TemporaryDirectory: fix recursion bug (seen in pythongh-79325,
Browse files Browse the repository at this point in the history
  • Loading branch information
kwi-dk committed Dec 1, 2022
1 parent 38f8356 commit 7e118a0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
6 changes: 4 additions & 2 deletions Lib/tempfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ def __init__(self, suffix=None, prefix=None, dir=None,
ignore_errors=self._ignore_cleanup_errors)

@classmethod
def _rmtree(cls, name, ignore_errors=False):
def _rmtree(cls, name, ignore_errors=False, retrying=None):
def without_following_symlinks(func, path, *args):
# Pass follow_symlinks=False, unless not supported on this platform.
if func in _os.supports_follow_symlinks:
Expand All @@ -889,7 +889,9 @@ def onerror(func, path, exc_info):
_os.unlink(path)
# PermissionError is raised on FreeBSD for directories
except (IsADirectoryError, PermissionError):
cls._rmtree(path, ignore_errors=ignore_errors)
if path == retrying:
raise # already tried (and failed) to fix this once
cls._rmtree(path, ignore_errors=ignore_errors, retrying=path)
except FileNotFoundError:
pass
elif issubclass(exc_info[0], FileNotFoundError):
Expand Down
24 changes: 24 additions & 0 deletions Lib/test/test_tempfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -1691,6 +1691,30 @@ def patched_unlink(path, **kwargs):
"Mode of the directory pointed to by a symlink changed")
d2.cleanup()

def test_cleanup_with_error_deleting_directory(self):
# cleanup() should not recurse infinitely on PermissionError when deleting directory
d = self.do_create()

# There are a variety of reasons why the OS may raise a PermissionError,
# but provoking those reliably and cross-platform is not straightforward,
# so raise the error synthetically instead.
real_rmdir = os.rmdir
error_was_raised = False
def patched_rmdir(path, **kwargs):
nonlocal error_was_raised
# rmdir may be called with full path or path relative to 'fd' kwarg.
if path.endswith("dir0"):
error_was_raised = True
raise PermissionError()
real_rmdir(path, **kwargs)

with mock.patch("tempfile._os.rmdir", patched_rmdir):
# This call to cleanup() should fail with PermissionError, not recurse infinitely.
self.assertRaises(PermissionError, d.cleanup)

self.assertTrue(error_was_raised, "did not see expected 'rmdir' call")
d.cleanup()

@support.cpython_only
def test_del_on_collection(self):
# A TemporaryDirectory is deleted when garbage collected
Expand Down

0 comments on commit 7e118a0

Please sign in to comment.