diff options
| -rw-r--r-- | Lib/shutil.py | 20 | ||||
| -rw-r--r-- | Lib/test/test_shutil.py | 35 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/Library/2020-12-14-09-31-13.bpo-35332.s22wAx.rst | 3 |
3 files changed, 56 insertions, 2 deletions
diff --git a/Lib/shutil.py b/Lib/shutil.py index bdbbcbc..dc3aac3 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -687,7 +687,12 @@ def _rmtree_safe_fd(topfd, path, onexc): _rmtree_safe_fd(dirfd, fullname, onexc) try: os.close(dirfd) + except OSError as err: + # close() should not be retried after an error. dirfd_closed = True + onexc(os.close, fullname, err) + dirfd_closed = True + try: os.rmdir(entry.name, dir_fd=topfd) except FileNotFoundError: continue @@ -704,7 +709,10 @@ def _rmtree_safe_fd(topfd, path, onexc): onexc(os.path.islink, fullname, err) finally: if not dirfd_closed: - os.close(dirfd) + try: + os.close(dirfd) + except OSError as err: + onexc(os.close, fullname, err) else: try: os.unlink(entry.name, dir_fd=topfd) @@ -782,7 +790,12 @@ def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None): _rmtree_safe_fd(fd, path, onexc) try: os.close(fd) + except OSError as err: + # close() should not be retried after an error. fd_closed = True + onexc(os.close, path, err) + fd_closed = True + try: os.rmdir(path, dir_fd=dir_fd) except OSError as err: onexc(os.rmdir, path, err) @@ -794,7 +807,10 @@ def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None): onexc(os.path.islink, path, err) finally: if not fd_closed: - os.close(fd) + try: + os.close(fd) + except OSError as err: + onexc(os.close, path, err) else: if dir_fd is not None: raise NotImplementedError("dir_fd unavailable on this platform") diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 9b8ec42..d7061b2 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -576,6 +576,41 @@ class TestRmTree(BaseTest, unittest.TestCase): self.assertFalse(shutil._use_fd_functions) self.assertFalse(shutil.rmtree.avoids_symlink_attacks) + @unittest.skipUnless(shutil._use_fd_functions, "requires safe rmtree") + def test_rmtree_fails_on_close(self): + # Test that the error handler is called for failed os.close() and that + # os.close() is only called once for a file descriptor. + tmp = self.mkdtemp() + dir1 = os.path.join(tmp, 'dir1') + os.mkdir(dir1) + dir2 = os.path.join(dir1, 'dir2') + os.mkdir(dir2) + def close(fd): + orig_close(fd) + nonlocal close_count + close_count += 1 + raise OSError + + close_count = 0 + with support.swap_attr(os, 'close', close) as orig_close: + with self.assertRaises(OSError): + shutil.rmtree(dir1) + self.assertTrue(os.path.isdir(dir2)) + self.assertEqual(close_count, 2) + + close_count = 0 + errors = [] + def onexc(*args): + errors.append(args) + with support.swap_attr(os, 'close', close) as orig_close: + shutil.rmtree(dir1, onexc=onexc) + self.assertEqual(len(errors), 2) + self.assertIs(errors[0][0], close) + self.assertEqual(errors[0][1], dir2) + self.assertIs(errors[1][0], close) + self.assertEqual(errors[1][1], dir1) + self.assertEqual(close_count, 2) + @unittest.skipUnless(shutil._use_fd_functions, "dir_fd is not supported") def test_rmtree_with_dir_fd(self): tmp_dir = self.mkdtemp() diff --git a/Misc/NEWS.d/next/Library/2020-12-14-09-31-13.bpo-35332.s22wAx.rst b/Misc/NEWS.d/next/Library/2020-12-14-09-31-13.bpo-35332.s22wAx.rst new file mode 100644 index 0000000..80564b9 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-12-14-09-31-13.bpo-35332.s22wAx.rst @@ -0,0 +1,3 @@ +The :func:`shutil.rmtree` function now ignores errors when calling +:func:`os.close` when *ignore_errors* is ``True``, and +:func:`os.close` no longer retried after error. |