GH-122890: Fix low-level error handling in `pathlib.Path.copy()` (#122897)

Give unique names to our low-level FD copying functions, and try each one
in turn. Handle errors appropriately for each implementation:

- `fcntl.FICLONE`: suppress `EBADF`, `EOPNOTSUPP`, `ETXTBSY`, `EXDEV`
- `posix._fcopyfile`: suppress `EBADF`, `ENOTSUP`
- `os.copy_file_range`: suppress `ETXTBSY`, `EXDEV`
- `os.sendfile`: suppress `ENOTSOCK`

diff --git a/Lib/pathlib/_os.py b/Lib/pathlib/_os.py
index 63dbe131baea519922a84ac1511db114e2b92e54..642b3a57c59a1d876a7336273116aa019e9b5698 100644 (file)
--- a/Lib/pathlib/_os.py
+++ b/Lib/pathlib/_os.py
@@ -20,7 +20,7 @@ except ImportError:
     _winapi = None
 
 
-def get_copy_blocksize(infd):
+def _get_copy_blocksize(infd):
     """Determine blocksize for fastcopying on Linux.
     Hopefully the whole file will be copied in a single call.
     The copying itself should be performed in a loop 'till EOF is
@@ -40,7 +40,7 @@ def get_copy_blocksize(infd):
 
 
 if fcntl and hasattr(fcntl, 'FICLONE'):
-    def clonefd(source_fd, target_fd):
+    def _ficlone(source_fd, target_fd):
         """
         Perform a lightweight copy of two files, where the data blocks are
         copied only when modified. This is known as Copy on Write (CoW),
@@ -48,18 +48,22 @@ if fcntl and hasattr(fcntl, 'FICLONE'):
         """
         fcntl.ioctl(target_fd, fcntl.FICLONE, source_fd)
 else:
-    clonefd = None
+    _ficlone = None
 
 
 if posix and hasattr(posix, '_fcopyfile'):
-    def copyfd(source_fd, target_fd):
+    def _fcopyfile(source_fd, target_fd):
         """
         Copy a regular file content using high-performance fcopyfile(3)
         syscall (macOS).
         """
         posix._fcopyfile(source_fd, target_fd, posix._COPYFILE_DATA)
-elif hasattr(os, 'copy_file_range'):
-    def copyfd(source_fd, target_fd):
+else:
+    _fcopyfile = None
+
+
+if hasattr(os, 'copy_file_range'):
+    def _copy_file_range(source_fd, target_fd):
         """
         Copy data from one regular mmap-like fd to another by using a
         high-performance copy_file_range(2) syscall that gives filesystems
@@ -67,7 +71,7 @@ elif hasattr(os, 'copy_file_range'):
         copy.
         This should work on Linux >= 4.5 only.
         """
-        blocksize = get_copy_blocksize(source_fd)
+        blocksize = _get_copy_blocksize(source_fd)
         offset = 0
         while True:
             sent = os.copy_file_range(source_fd, target_fd, blocksize,
@@ -75,13 +79,17 @@ elif hasattr(os, 'copy_file_range'):
             if sent == 0:
                 break  # EOF
             offset += sent
-elif hasattr(os, 'sendfile'):
-    def copyfd(source_fd, target_fd):
+else:
+    _copy_file_range = None
+
+
+if hasattr(os, 'sendfile'):
+    def _sendfile(source_fd, target_fd):
         """Copy data from one regular mmap-like fd to another by using
         high-performance sendfile(2) syscall.
         This should work on Linux >= 2.6.33 only.
         """
-        blocksize = get_copy_blocksize(source_fd)
+        blocksize = _get_copy_blocksize(source_fd)
         offset = 0
         while True:
             sent = os.sendfile(target_fd, source_fd, offset, blocksize)
@@ -89,7 +97,7 @@ elif hasattr(os, 'sendfile'):
                 break  # EOF
             offset += sent
 else:
-    copyfd = None
+    _sendfile = None
 
 
 if _winapi and hasattr(_winapi, 'CopyFile2'):
@@ -114,18 +122,36 @@ def copyfileobj(source_f, target_f):
     else:
         try:
             # Use OS copy-on-write where available.
-            if clonefd:
+            if _ficlone:
                 try:
-                    clonefd(source_fd, target_fd)
+                    _ficlone(source_fd, target_fd)
                     return
                 except OSError as err:
                     if err.errno not in (EBADF, EOPNOTSUPP, ETXTBSY, EXDEV):
                         raise err
 
             # Use OS copy where available.
-            if copyfd:
-                copyfd(source_fd, target_fd)
-                return
+            if _fcopyfile:
+                try:
+                    _fcopyfile(source_fd, target_fd)
+                    return
+                except OSError as err:
+                    if err.errno not in (EINVAL, ENOTSUP):
+                        raise err
+            if _copy_file_range:
+                try:
+                    _copy_file_range(source_fd, target_fd)
+                    return
+                except OSError as err:
+                    if err.errno not in (ETXTBSY, EXDEV):
+                        raise err
+            if _sendfile:
+                try:
+                    _sendfile(source_fd, target_fd)
+                    return
+                except OSError as err:
+                    if err.errno != ENOTSOCK:
+                        raise err
         except OSError as err:
             # Produce more useful error messages.
             err.filename = source_f.name

diff --git a/Lib/test/test_pathlib/test_pathlib.py b/Lib/test/test_pathlib/test_pathlib.py
index 4c60f278834c9c43f5ec40af1b30d9f0c69a73b5..ad1720cdb24f0bac53de400e2c6fbde870216cb8 100644 (file)
--- a/Lib/test/test_pathlib/test_pathlib.py
+++ b/Lib/test/test_pathlib/test_pathlib.py
@@ -1,3 +1,4 @@
+import contextlib
 import io
 import os
 import sys
@@ -22,10 +23,18 @@ from test.support.os_helper import TESTFN, FakePath
 from test.test_pathlib import test_pathlib_abc
 from test.test_pathlib.test_pathlib_abc import needs_posix, needs_windows, needs_symlinks
 
+try:
+    import fcntl
+except ImportError:
+    fcntl = None
 try:
     import grp, pwd
 except ImportError:
     grp = pwd = None
+try:
+    import posix
+except ImportError:
+    posix = None
 
 
 root_in_posix = False
@@ -707,6 +716,45 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest):
         if hasattr(source_st, 'st_flags'):
             self.assertEqual(source_st.st_flags, target_st.st_flags)
 
+    def test_copy_error_handling(self):
+        def make_raiser(err):
+            def raiser(*args, **kwargs):
+                raise OSError(err, os.strerror(err))
+            return raiser
+
+        base = self.cls(self.base)
+        source = base / 'fileA'
+        target = base / 'copyA'
+
+        # Raise non-fatal OSError from all available fast copy functions.
+        with contextlib.ExitStack() as ctx:
+            if fcntl and hasattr(fcntl, 'FICLONE'):
+                ctx.enter_context(mock.patch('fcntl.ioctl', make_raiser(errno.EXDEV)))
+            if posix and hasattr(posix, '_fcopyfile'):
+                ctx.enter_context(mock.patch('posix._fcopyfile', make_raiser(errno.ENOTSUP)))
+            if hasattr(os, 'copy_file_range'):
+                ctx.enter_context(mock.patch('os.copy_file_range', make_raiser(errno.EXDEV)))
+            if hasattr(os, 'sendfile'):
+                ctx.enter_context(mock.patch('os.sendfile', make_raiser(errno.ENOTSOCK)))
+
+            source.copy(target)
+            self.assertTrue(target.exists())
+            self.assertEqual(source.read_text(), target.read_text())
+
+        # Raise fatal OSError from first available fast copy function.
+        if fcntl and hasattr(fcntl, 'FICLONE'):
+            patchpoint = 'fcntl.ioctl'
+        elif posix and hasattr(posix, '_fcopyfile'):
+            patchpoint = 'posix._fcopyfile'
+        elif hasattr(os, 'copy_file_range'):
+            patchpoint = 'os.copy_file_range'
+        elif hasattr(os, 'sendfile'):
+            patchpoint = 'os.sendfile'
+        else:
+            return
+        with mock.patch(patchpoint, make_raiser(errno.ENOENT)):
+            self.assertRaises(FileNotFoundError, source.copy, target)
+
     @unittest.skipIf(sys.platform == "win32" or sys.platform == "wasi", "directories are always readable on Windows and WASI")
     @unittest.skipIf(root_in_posix, "test fails with root privilege")
     def test_copy_dir_no_read_permission(self):