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`
This commit is contained in:
Barney Gale 2024-08-24 15:11:39 +01:00 committed by GitHub
parent 127660bcdb
commit c4ee4e756a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 90 additions and 16 deletions

View file

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

View file

@ -1,3 +1,4 @@
import contextlib
import io import io
import os import os
import sys 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 import test_pathlib_abc
from test.test_pathlib.test_pathlib_abc import needs_posix, needs_windows, needs_symlinks from test.test_pathlib.test_pathlib_abc import needs_posix, needs_windows, needs_symlinks
try:
import fcntl
except ImportError:
fcntl = None
try: try:
import grp, pwd import grp, pwd
except ImportError: except ImportError:
grp = pwd = None grp = pwd = None
try:
import posix
except ImportError:
posix = None
root_in_posix = False root_in_posix = False
@ -707,6 +716,45 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest):
if hasattr(source_st, 'st_flags'): if hasattr(source_st, 'st_flags'):
self.assertEqual(source_st.st_flags, target_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(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") @unittest.skipIf(root_in_posix, "test fails with root privilege")
def test_copy_dir_no_read_permission(self): def test_copy_dir_no_read_permission(self):