From 5b251a5fecda927d37fcc238887362328c5c40b5 Mon Sep 17 00:00:00 2001 From: dot Date: Sat, 10 Oct 2026 19:44:05 +0900 Subject: [PATCH] Implement mkfifo with directory descriptor support Support directory-relative FIFO creation. Retry interrupted path opens while releasing the interpreter and honoring signal-handler exceptions; preserve raw directory descriptors across callbacks. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/host_env/src/crt_fd.rs | 8 +- crates/host_env/src/posix.rs | 61 +++++++++++++++ crates/vm/src/stdlib/_io.rs | 19 +++-- crates/vm/src/stdlib/os.rs | 118 +++++++++++++++++++++------- crates/vm/src/stdlib/posix.rs | 92 ++++++++++++++++++++++ extra_tests/snippets/stdlib_os.py | 125 ++++++++++++++++++++++++++++++ 6 files changed, 385 insertions(+), 38 deletions(-) diff --git a/crates/host_env/src/crt_fd.rs b/crates/host_env/src/crt_fd.rs index cdedf177b04..c9af646122f 100644 --- a/crates/host_env/src/crt_fd.rs +++ b/crates/host_env/src/crt_fd.rs @@ -331,7 +331,13 @@ pub fn wopen(path: &widestring::WideCStr, flags: i32, mode: i32) -> io::Result, path: &ffi::CStr, flags: i32, mode: i32) -> io::Result { - cvt_fd(unsafe { c::openat(dir.as_raw(), path.as_ptr(), flags, mode) }) + openat_raw(dir.as_raw(), path, flags, mode) +} + +#[cfg(all(any(unix, target_os = "wasi"), not(target_os = "redox")))] +// Accept a descriptor number without asserting it stays open across calls. +pub fn openat_raw(dir: Raw, path: &ffi::CStr, flags: i32, mode: i32) -> io::Result { + cvt_fd(unsafe { c::openat(dir, path.as_ptr(), flags, mode) }) } pub fn fsync(fd: Borrowed<'_>) -> io::Result<()> { diff --git a/crates/host_env/src/posix.rs b/crates/host_env/src/posix.rs index ae7c0d3dedc..053148ca4a4 100644 --- a/crates/host_env/src/posix.rs +++ b/crates/host_env/src/posix.rs @@ -569,6 +569,67 @@ pub fn statvfs_fd(fd: i32) -> std::io::Result { } } +#[cfg(not(target_os = "redox"))] +pub fn mkfifo(path: &CStr, mode: libc::mode_t) -> std::io::Result<()> { + let ret = unsafe { libc::mkfifo(path.as_ptr(), mode) }; + if ret == 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } +} + +#[cfg(target_vendor = "apple")] +type MkfifoAt = unsafe extern "C" fn(libc::c_int, *const libc::c_char, libc::mode_t) -> libc::c_int; + +#[cfg(target_vendor = "apple")] +fn mkfifoat_symbol() -> Option { + static MKFIFOAT: std::sync::OnceLock> = std::sync::OnceLock::new(); + *MKFIFOAT.get_or_init(|| { + // mkfifoat was added in macOS 13 / iOS 16. Resolve it at runtime so + // ordinary mkfifo remains usable on older Apple systems. + let symbol = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c"mkfifoat".as_ptr()) }; + if symbol.is_null() { + None + } else { + // SAFETY: the system symbol has the MkfifoAt signature and stays + // loaded for the lifetime of the process. + Some(unsafe { core::mem::transmute::<*mut libc::c_void, MkfifoAt>(symbol) }) + } + }) +} + +#[cfg(not(target_os = "redox"))] +pub fn has_mkfifoat() -> bool { + #[cfg(target_vendor = "apple")] + { + mkfifoat_symbol().is_some() + } + #[cfg(not(target_vendor = "apple"))] + { + !cfg!(target_os = "android") + } +} + +#[cfg(not(any(target_os = "redox", target_os = "android")))] +pub fn mkfifoat(dir_fd: i32, path: &CStr, mode: libc::mode_t) -> std::io::Result<()> { + #[cfg(target_vendor = "apple")] + let mkfifoat = mkfifoat_symbol().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::Unsupported, + "mkfifoat unavailable on this platform", + ) + })?; + #[cfg(not(target_vendor = "apple"))] + let mkfifoat = libc::mkfifoat; + let ret = unsafe { mkfifoat(dir_fd, path.as_ptr(), mode) }; + if ret == 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } +} + #[cfg(not(target_os = "redox"))] pub fn mknod(path: &CStr, mode: libc::mode_t, device: libc::dev_t) -> std::io::Result<()> { let ret = unsafe { libc::mknod(path.as_ptr(), mode, device) }; diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index 1d86717a211..5bcd8f64d29 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -5930,7 +5930,7 @@ mod _io { #[pymodule] mod fileio { use super::{_io::*, Offset, iobase_finalize}; - use crate::host_env::crt_fd; + use crate::host_env::{crt_fd, os::ErrorExt}; use crate::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyResult, TryFromObject, VirtualMachine, builtins::{PyBaseExceptionRef, PyUtf8Str, PyUtf8StrRef}, @@ -6044,14 +6044,19 @@ mod fileio { } else { let path = OsPath::try_from_fspath(name.clone(), vm)?; #[cfg(any(unix, target_os = "wasi"))] - let fd = host_io::open_path(&path.clone().into_cstring(vm)?, flags, 0o666); + let raw_path = path.clone().into_cstring(vm)?; #[cfg(windows)] - let fd = host_io::open_path(&path.to_wide_cstring(vm)?, flags, 0o666); + let raw_path = path.to_wide_cstring(vm)?; let filename = OsPathOrFd::Path(path); - match fd { - Ok(fd) => (fd.into_raw(), Some(filename)), - Err(e) => { - return Err(OSErrorBuilder::with_filename_from_errno(&e, filename, vm)); + loop { + match vm.allow_threads(|| host_io::open_path(&raw_path, flags, 0o666)) { + Ok(fd) => break (fd.into_raw(), Some(filename)), + Err(e) if e.posix_errno() == libc::EINTR => vm.check_signals()?, + Err(e) => { + return Err(OSErrorBuilder::with_filename_from_errno( + &e, filename, vm, + )); + } } } } diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index b873cb4c251..7419c8f9260 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -199,7 +199,10 @@ impl ToPyObject for crt_fd::Borrowed<'_> { #[pymodule(sub, name = "posix")] pub(super) mod _os { - use super::{DirFd, DstDirFd, FollowSymlinks, RawMode, SrcDirFd, SupportFunc}; + use super::{ + AT_FDCWD, DefaultDirFd, DirFd, DirFdKeyword, DstDirFd, FollowSymlinks, RawMode, SrcDirFd, + SupportFunc, warn_if_bool_fd, + }; #[cfg(not(windows))] use crate::exceptions; use crate::host_env::fileutils::StatStruct; @@ -212,9 +215,11 @@ pub(super) mod _os { }, class::PyClassDef, common::lock::{OnceCell, PyRwLock}, - convert::{IntoPyException, ToPyObject}, + convert::{IntoPyException, ToPyException, ToPyObject}, exceptions::{OSErrorBuilder, ToOSErrorBuilder}, - function::{ArgBytesLike, ArgMemoryBuffer, FsPath, FuncArgs}, + function::{ + ArgBytesLike, ArgMemoryBuffer, ArgumentError, FromArgs, FsPath, FuncArgs, Param, + }, host_env::crt_fd, ospath::{OsPath, OsPathOrFd, OutputMode, PathConverter}, protocol::PyIterReturn, @@ -276,20 +281,57 @@ pub(super) mod _os { crt_fd::closerange(fd_low, fd_high); } + // Signal callbacks can close this descriptor between open attempts. + #[cfg(any(unix, windows, target_os = "wasi"))] + struct OpenDirFd(crt_fd::Raw); + + #[cfg(any(unix, windows, target_os = "wasi"))] + impl FromArgs for OpenDirFd { + const PARAMS: Option<&'static [Param]> = Some(DefaultDirFd::PARAMS); + + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { + let fd = match args.take_keyword("dir_fd") { + Some(o) if vm.is_none(&o) => AT_FDCWD, + None => AT_FDCWD, + Some(o) => { + warn_if_bool_fd(&o, vm).map_err(Into::::into)?; + let fd = o.try_index_opt(vm).unwrap_or_else(|| { + Err(vm.new_type_error(format!( + "argument should be integer or None, not {}", + o.class().name() + ))) + })?; + fd.try_to_primitive(vm)? + } + }; + if fd == -1 { + return Err(io::Error::from_raw_os_error(libc::EBADF) + .to_pyexception(vm) + .into()); + } + if !OPEN_DIR_FD && fd != AT_FDCWD { + return Err(vm + .new_not_implemented_error("dir_fd unavailable on this platform") + .into()); + } + Ok(Self(fd)) + } + } + #[cfg(any(unix, windows, target_os = "wasi"))] #[derive(FromArgs)] - struct OpenArgs<'fd> { + struct OpenArgs { path: OsPath, flags: i32, #[pyarg(any, default = 0o777)] mode: i32, #[pyarg(flatten)] - dir_fd: DirFd<'fd, { OPEN_DIR_FD as usize }>, + dir_fd: OpenDirFd, } #[pyfunction] - fn open(args: OpenArgs<'_>, vm: &VirtualMachine) -> PyResult { - os_open(args.path, args.flags, args.mode, args.dir_fd, vm) + fn open(args: OpenArgs, vm: &VirtualMachine) -> PyResult { + os_open(args.path, args.flags, args.mode, args.dir_fd.0, vm) } #[cfg(any(unix, windows, target_os = "wasi"))] @@ -297,34 +339,50 @@ pub(super) mod _os { name: OsPath, flags: i32, mode: i32, - dir_fd: DirFd<'_, { OPEN_DIR_FD as usize }>, + dir_fd: crt_fd::Raw, vm: &VirtualMachine, ) -> PyResult { + use crate::host_env::os::ErrorExt; + #[cfg(windows)] - let fd = { - let [] = dir_fd.0; - let name = name.to_wide_cstring(vm)?; - let flags = flags | crate::host_env::os::O_NOINHERIT; - crt_fd::wopen(&name, flags, mode) - }; + let raw_name = name.to_wide_cstring(vm)?; #[cfg(not(windows))] - let fd = { - let name = name.clone().into_cstring(vm)?; - #[cfg(not(target_os = "wasi"))] - let flags = flags | crate::host_env::os::O_CLOEXEC; - #[cfg(not(target_os = "redox"))] - if let Some(dir_fd) = dir_fd.get_opt() { - crt_fd::openat(dir_fd, &name, flags, mode) - } else { - crt_fd::open(&name, flags, mode) - } - #[cfg(target_os = "redox")] - { - let [] = dir_fd.0; - crt_fd::open(&name, flags, mode) + let raw_name = name.clone().into_cstring(vm)?; + #[cfg(windows)] + let flags = flags | crate::host_env::os::O_NOINHERIT; + #[cfg(all(not(windows), not(target_os = "wasi")))] + let flags = flags | crate::host_env::os::O_CLOEXEC; + + loop { + let fd = vm.allow_threads(|| { + #[cfg(windows)] + { + let _ = dir_fd; + crt_fd::wopen(&raw_name, flags, mode) + } + #[cfg(not(windows))] + { + #[cfg(not(target_os = "redox"))] + if dir_fd != AT_FDCWD { + crt_fd::openat_raw(dir_fd, &raw_name, flags, mode) + } else { + crt_fd::open(&raw_name, flags, mode) + } + #[cfg(target_os = "redox")] + { + let _ = dir_fd; + crt_fd::open(&raw_name, flags, mode) + } + } + }); + match fd { + Ok(fd) => return Ok(fd), + Err(err) if err.posix_errno() == libc::EINTR => vm.check_signals()?, + Err(err) => { + return Err(OSErrorBuilder::with_filename_from_errno(&err, name, vm)); + } } - }; - fd.map_err(|err| OSErrorBuilder::with_filename_from_errno(&err, name, vm)) + } } #[derive(FromArgs)] diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index 96bb0066702..388771acbc6 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -1012,6 +1012,91 @@ pub mod module { } } + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + struct MkfifoPath(OsPath); + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + impl TryFromObject for MkfifoPath { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + crate::ospath::PathConverter::new() + .function("mkfifo") + .try_path(obj, vm) + .map(Self) + } + } + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + struct MkfifoMode(i32); + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + impl TryFromObject for MkfifoMode { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + let mode = obj.try_index(vm)?; + i32::try_from(mode.as_bigint()) + .map(Self) + .map_err(|_| vm.new_overflow_error("Python int too large to convert to C int")) + } + } + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + #[derive(FromArgs)] + struct MkfifoArgs { + #[pyarg(any)] + path: MkfifoPath, + #[pyarg(any, default = MkfifoMode(0o666), py_default = "438")] + mode: MkfifoMode, + #[pyarg(named, optional, py_default = "None")] + dir_fd: OptionalArg, + } + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + #[pyfunction] + fn mkfifo(args: MkfifoArgs, vm: &VirtualMachine) -> PyResult<()> { + let c_path = args.path.0.into_cstring(vm)?; + let mode = args.mode.0 as libc::mode_t; + // Keep a raw C int: even -1 is valid with an absolute path, for which + // mkfifoat ignores dir_fd. BorrowedFd cannot represent that value. + let dir_fd = match args.dir_fd { + OptionalArg::Present(fd) if !vm.is_none(&fd) => { + warn_if_bool_fd(&fd, vm)?; + let fd = fd.try_index_opt(vm).unwrap_or_else(|| { + Err(vm.new_type_error(format!( + "argument should be integer or None, not {}", + fd.class().name() + ))) + })?; + let fd = i32::try_from(fd.as_bigint()).map_err(|_| { + let message = if fd.as_bigint().sign() == malachite_bigint::Sign::Minus { + "fd is less than minimum" + } else { + "fd is greater than maximum" + }; + vm.new_overflow_error(message) + })?; + (fd != rustpython_host_env::os::AT_FDCWD).then_some(fd) + } + _ => None, + }; + if dir_fd.is_some() && !host_posix::has_mkfifoat() { + return Err(vm.new_not_implemented_error("dir_fd unavailable on this platform")); + } + + loop { + let result = vm.allow_threads(|| { + #[cfg(not(target_os = "android"))] + if let Some(dir_fd) = dir_fd { + return host_posix::mkfifoat(dir_fd, &c_path, mode); + } + host_posix::mkfifo(&c_path, mode) + }); + match result { + Ok(()) => return Ok(()), + Err(err) if err.raw_os_error() == Some(libc::EINTR) => vm.check_signals()?, + Err(err) => return Err(err.into_pyexception(vm)), + } + } + } + #[cfg(not(target_os = "redox"))] const MKNOD_DIR_FD: bool = cfg!(not(target_vendor = "apple")); @@ -2243,6 +2328,13 @@ pub mod module { SupportFunc::new("lchown", None, None, None), #[cfg(not(target_os = "redox"))] SupportFunc::new("fchown", Some(true), None, Some(true)), + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + SupportFunc::new( + "mkfifo", + Some(false), + Some(host_posix::has_mkfifoat()), + Some(false), + ), #[cfg(not(target_os = "redox"))] SupportFunc::new("mknod", Some(true), Some(MKNOD_DIR_FD), Some(false)), SupportFunc::new("umask", Some(false), Some(false), Some(false)), diff --git a/extra_tests/snippets/stdlib_os.py b/extra_tests/snippets/stdlib_os.py index df7c723c967..50ce16be191 100644 --- a/extra_tests/snippets/stdlib_os.py +++ b/extra_tests/snippets/stdlib_os.py @@ -633,3 +633,128 @@ def __exit__(self, exc_type, exc_val, exc_tb): assert os.stat_result(tuple(range(10)), {"st_atime": 1.5}).st_atime == 1.5 with assert_raises(TypeError): os.stat_result(tuple(range(10)), ["st_atime"]) + + +def mkfifo_paths_and_directory_descriptors(): + import errno + import inspect + import tempfile + import warnings + + assert str(inspect.signature(os.mkfifo)) == "(path, mode=438, *, dir_fd=None)" + assert os.mkfifo not in os.supports_fd + assert os.mkfifo not in os.supports_follow_symlinks + if sys.platform.startswith("linux"): + assert os.mkfifo in os.supports_dir_fd + + class Path: + def __init__(self, value): + self.value = value + + def __fspath__(self): + return self.value + + class Index: + def __init__(self, value): + self.value = value + + def __index__(self): + return self.value + + class BadIndex: + def __index__(self): + raise RuntimeError("mkfifo index marker") + + # Never open a FIFO: creating, inspecting and unlinking it cannot wait for + # another process to connect. All paths belong to this temporary directory. + with tempfile.TemporaryDirectory(prefix="rustpython-mkfifo-") as directory: + path = os.path.join(directory, "fifo") + for value in (path, os.fsencode(path), Path(path), Path(os.fsencode(path))): + assert os.mkfifo(path=value, mode=0) is None + info = os.stat(path) + assert stat.S_ISFIFO(info.st_mode) + assert stat.S_IMODE(info.st_mode) == 0 + with assert_raises(FileExistsError) as error: + os.mkfifo(value) + assert error.exception.errno == errno.EEXIST + assert error.exception.filename is None + os.unlink(path) + + assert os.mkfifo(path, dir_fd=None) is None + assert stat.S_ISFIFO(os.stat(path).st_mode) + assert stat.S_IMODE(os.stat(path).st_mode) & ~0o666 == 0 + os.unlink(path) + + for value in (None, 1.5): + assert_raises(TypeError, os.mkfifo, path, value) + for value in (2**31, -(2**31) - 1, 2**100, -(2**100)): + with assert_raises(OverflowError) as error: + os.mkfifo(path, value) + assert str(error.exception) == "Python int too large to convert to C int" + with assert_raises(OverflowError) as error: + os.mkfifo(path, dir_fd=value) + expected = ( + "fd is greater than maximum" if value > 0 else "fd is less than minimum" + ) + assert str(error.exception) == expected + for value in (1.5, "fd"): + assert_raises(TypeError, os.mkfifo, path, dir_fd=value) + for value in (None, 1): + with assert_raises(TypeError) as error: + os.mkfifo(value) + expected = "NoneType" if value is None else "int" + assert str(error.exception) == ( + "mkfifo: path should be string, bytes or os.PathLike, not " + expected + ) + assert_raises(TypeError, os.mkfifo, Path(None)) + for value in (path + "\0", os.fsencode(path) + b"\0"): + with assert_raises(ValueError) as error: + os.mkfifo(value) + assert str(error.exception) == "mkfifo: embedded null character in path" + assert_raises(TypeError, os.mkfifo, path, 0o600, None) + for options in ({"mode": BadIndex()}, {"dir_fd": BadIndex()}): + with assert_raises(RuntimeError) as error: + os.mkfifo(path, **options) + assert str(error.exception) == "mkfifo index marker" + with assert_raises(FileNotFoundError) as error: + os.mkfifo(os.path.join(directory, "missing", "fifo")) + assert error.exception.errno == errno.ENOENT + assert error.exception.filename is None + + directory_fd = os.open(directory, os.O_RDONLY) + try: + if os.mkfifo in os.supports_dir_fd: + assert ( + os.mkfifo(Path(b"fifo"), Index(0o600), dir_fd=Index(directory_fd)) + is None + ) + assert stat.S_ISFIFO(os.stat(path).st_mode) + assert stat.S_IMODE(os.stat(path).st_mode) & ~0o600 == 0 + os.unlink(path) + for value in (-1, -(2**31)): + assert os.mkfifo(path, dir_fd=value) is None + assert stat.S_ISFIFO(os.stat(path).st_mode) + os.unlink(path) + with assert_raises(OSError) as error: + os.mkfifo("fifo", dir_fd=-1) + assert error.exception.errno == errno.EBADF + assert error.exception.filename is None + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + assert os.mkfifo(path, dir_fd=True) is None + assert len(records) == 1 + assert records[0].category is RuntimeWarning + assert str(records[0].message) == "bool is used as a file descriptor" + os.unlink(path) + else: + with assert_raises(NotImplementedError) as error: + os.mkfifo(path, dir_fd=directory_fd) + assert str(error.exception) == "dir_fd unavailable on this platform" + finally: + os.close(directory_fd) + + +if sys.platform.startswith("linux"): + assert hasattr(os, "mkfifo") +if hasattr(os, "mkfifo"): + mkfifo_paths_and_directory_descriptors()