gh-81441: shutil.rmtree() FileNotFoundError race condition (GH-14064)

Ignore missing files and directories while enumerating
directory entries in shutil.rmtree().

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst
index d1949d698f5614497b9386dba87ce513418dc0f9..d30d289710b129a5bc01cb4f41a0fc5066e8b5e4 100644 (file)
--- a/Doc/library/shutil.rst
+++ b/Doc/library/shutil.rst
@@ -343,6 +343,10 @@ Directory and files operations
    .. versionchanged:: 3.12
       Added the *onexc* parameter, deprecated *onerror*.
 
+   .. versionchanged:: 3.13
+      :func:`!rmtree` now ignores :exc:`FileNotFoundError` exceptions for all
+      but the top-level path.
+
    .. attribute:: rmtree.avoids_symlink_attacks
 
       Indicates whether the current platform and implementation provides a

diff --git a/Lib/shutil.py b/Lib/shutil.py
index dd93872e83c9e2c10ed01b7af10a15a56732fa37..93b00d73a0fd46717040a57de31160319d91791a 100644 (file)
--- a/Lib/shutil.py
+++ b/Lib/shutil.py
@@ -590,23 +590,21 @@ def copytree(src, dst, symlinks=False, ignore=None, copy_function=copy2,
                      dirs_exist_ok=dirs_exist_ok)
 
 if hasattr(os.stat_result, 'st_file_attributes'):
-    def _rmtree_islink(path):
-        try:
-            st = os.lstat(path)
-            return (stat.S_ISLNK(st.st_mode) or
-                (st.st_file_attributes & stat.FILE_ATTRIBUTE_REPARSE_POINT
-                 and st.st_reparse_tag == stat.IO_REPARSE_TAG_MOUNT_POINT))
-        except OSError:
-            return False
+    def _rmtree_islink(st):
+        return (stat.S_ISLNK(st.st_mode) or
+            (st.st_file_attributes & stat.FILE_ATTRIBUTE_REPARSE_POINT
+                and st.st_reparse_tag == stat.IO_REPARSE_TAG_MOUNT_POINT))
 else:
-    def _rmtree_islink(path):
-        return os.path.islink(path)
+    def _rmtree_islink(st):
+        return stat.S_ISLNK(st.st_mode)
 
 # version vulnerable to race conditions
 def _rmtree_unsafe(path, onexc):
     try:
         with os.scandir(path) as scandir_it:
             entries = list(scandir_it)
+    except FileNotFoundError:
+        return
     except OSError as err:
         onexc(os.scandir, path, err)
         entries = []
@@ -614,6 +612,8 @@ def _rmtree_unsafe(path, onexc):
         fullname = entry.path
         try:
             is_dir = entry.is_dir(follow_symlinks=False)
+        except FileNotFoundError:
+            continue
         except OSError:
             is_dir = False
 
@@ -624,6 +624,8 @@ def _rmtree_unsafe(path, onexc):
                     # a directory with a symlink after the call to
                     # os.scandir or entry.is_dir above.
                     raise OSError("Cannot call rmtree on a symbolic link")
+            except FileNotFoundError:
+                continue
             except OSError as err:
                 onexc(os.path.islink, fullname, err)
                 continue
@@ -631,10 +633,14 @@ def _rmtree_unsafe(path, onexc):
         else:
             try:
                 os.unlink(fullname)
+            except FileNotFoundError:
+                continue
             except OSError as err:
                 onexc(os.unlink, fullname, err)
     try:
         os.rmdir(path)
+    except FileNotFoundError:
+        pass
     except OSError as err:
         onexc(os.rmdir, path, err)
 
@@ -643,6 +649,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
     try:
         with os.scandir(topfd) as scandir_it:
             entries = list(scandir_it)
+    except FileNotFoundError:
+        return
     except OSError as err:
         err.filename = path
         onexc(os.scandir, path, err)
@@ -651,6 +659,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
         fullname = os.path.join(path, entry.name)
         try:
             is_dir = entry.is_dir(follow_symlinks=False)
+        except FileNotFoundError:
+            continue
         except OSError:
             is_dir = False
         else:
@@ -658,6 +668,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
                 try:
                     orig_st = entry.stat(follow_symlinks=False)
                     is_dir = stat.S_ISDIR(orig_st.st_mode)
+                except FileNotFoundError:
+                    continue
                 except OSError as err:
                     onexc(os.lstat, fullname, err)
                     continue
@@ -665,6 +677,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
             try:
                 dirfd = os.open(entry.name, os.O_RDONLY, dir_fd=topfd)
                 dirfd_closed = False
+            except FileNotFoundError:
+                continue
             except OSError as err:
                 onexc(os.open, fullname, err)
             else:
@@ -675,6 +689,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
                             os.close(dirfd)
                             dirfd_closed = True
                             os.rmdir(entry.name, dir_fd=topfd)
+                        except FileNotFoundError:
+                            continue
                         except OSError as err:
                             onexc(os.rmdir, fullname, err)
                     else:
@@ -692,6 +708,8 @@ def _rmtree_safe_fd(topfd, path, onexc):
         else:
             try:
                 os.unlink(entry.name, dir_fd=topfd)
+            except FileNotFoundError:
+                continue
             except OSError as err:
                 onexc(os.unlink, fullname, err)
 
@@ -781,7 +799,12 @@ def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None):
         if dir_fd is not None:
             raise NotImplementedError("dir_fd unavailable on this platform")
         try:
-            if _rmtree_islink(path):
+            st = os.lstat(path)
+        except OSError as err:
+            onexc(os.lstat, path, err)
+            return
+        try:
+            if _rmtree_islink(st):
                 # symlinks to directories are forbidden, see bug #1669
                 raise OSError("Cannot call rmtree on a symbolic link")
         except OSError as err:

diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index ae6c6814fcc3eca6b698176c5d5a6fbc191f0f9d..7ea2496230da476b85adb369d1e63e21166736e6 100644 (file)
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -633,6 +633,63 @@ class TestRmTree(BaseTest, unittest.TestCase):
         finally:
             shutil.rmtree(TESTFN, ignore_errors=True)
 
+    @unittest.skipIf(sys.platform[:6] == 'cygwin',
+                     "This test can't be run on Cygwin (issue #1071513).")
+    @os_helper.skip_if_dac_override
+    @os_helper.skip_unless_working_chmod
+    def test_rmtree_deleted_race_condition(self):
+        # bpo-37260
+        #
+        # Test that a file or a directory deleted after it is enumerated
+        # by scandir() but before unlink() or rmdr() is called doesn't
+        # generate any errors.
+        def _onexc(fn, path, exc):
+            assert fn in (os.rmdir, os.unlink)
+            if not isinstance(exc, PermissionError):
+                raise
+            # Make the parent and the children writeable.
+            for p, mode in zip(paths, old_modes):
+                os.chmod(p, mode)
+            # Remove other dirs except one.
+            keep = next(p for p in dirs if p != path)
+            for p in dirs:
+                if p != keep:
+                    os.rmdir(p)
+            # Remove other files except one.
+            keep = next(p for p in files if p != path)
+            for p in files:
+                if p != keep:
+                    os.unlink(p)
+
+        os.mkdir(TESTFN)
+        paths = [TESTFN] + [os.path.join(TESTFN, f'child{i}')
+                            for i in range(6)]
+        dirs = paths[1::2]
+        files = paths[2::2]
+        for path in dirs:
+            os.mkdir(path)
+        for path in files:
+            write_file(path, '')
+
+        old_modes = [os.stat(path).st_mode for path in paths]
+
+        # Make the parent and the children non-writeable.
+        new_mode = stat.S_IREAD|stat.S_IEXEC
+        for path in reversed(paths):
+            os.chmod(path, new_mode)
+
+        try:
+            shutil.rmtree(TESTFN, onexc=_onexc)
+        except:
+            # Test failed, so cleanup artifacts.
+            for path, mode in zip(paths, old_modes):
+                try:
+                    os.chmod(path, mode)
+                except OSError:
+                    pass
+            shutil.rmtree(TESTFN)
+            raise
+
 
 class TestCopyTree(BaseTest, unittest.TestCase):
 

diff --git a/Misc/NEWS.d/next/Library/2019-06-14-22-37-32.bpo-37260.oecdIf.rst b/Misc/NEWS.d/next/Library/2019-06-14-22-37-32.bpo-37260.oecdIf.rst
new file mode 100644 (file)
index 0000000..a5f2c5e
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-06-14-22-37-32.bpo-37260.oecdIf.rst
@@ -0,0 +1,2 @@
+Fixed a race condition in :func:`shutil.rmtree` in which directory entries removed by another process or thread while ``shutil.rmtree()`` is running can cause it to raise FileNotFoundError.  Patch by Jeffrey Kintscher.
+