Implement mkfifo with directory descriptor support (#9056)

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

@@ -331,7 +331,13 @@ pub fn wopen(path: &widestring::WideCStr, flags: i32, mode: i32) -> io::Result<O
 
 #[cfg(all(any(unix, target_os = "wasi"), not(target_os = "redox")))]
 pub fn openat(dir: Borrowed<'_>, path: &ffi::CStr, flags: i32, mode: i32) -> io::Result<Owned> {
-    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<Owned> {
+    cvt_fd(unsafe { c::openat(dir, path.as_ptr(), flags, mode) })
 }
 
 pub fn fsync(fd: Borrowed<'_>) -> io::Result<()> {

crates/host_env/src/posix.rs

@@ -569,6 +569,67 @@ pub fn statvfs_fd(fd: i32) -> std::io::Result<StatVfsInfo> {
     }
 }
 
+#[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<MkfifoAt> {
+    static MKFIFOAT: std::sync::OnceLock<Option<MkfifoAt>> = 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) };

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,
+                                ));
+                            }
                         }
                     }
                 }

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<Self, ArgumentError> {
+            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::<ArgumentError>::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<crt_fd::Owned> {
-        os_open(args.path, args.flags, args.mode, args.dir_fd, vm)
+    fn open(args: OpenArgs, vm: &VirtualMachine) -> PyResult<crt_fd::Owned> {
+        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<crt_fd::Owned> {
+        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)
+        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));
+                }
             }
-            #[cfg(target_os = "redox")]
-            {
-                let [] = dir_fd.0;
-                crt_fd::open(&name, flags, mode)
-            }
-        };
-        fd.map_err(|err| OSErrorBuilder::with_filename_from_errno(&err, name, vm))
+        }
     }
 
     #[derive(FromArgs)]

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<Self> {
+            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<Self> {
+            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<PyObjectRef>,
+    }
+
+    #[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)),

extra_tests/snippets/stdlib_os.py

@@ -633,3 +633,128 @@ assert os.stat_result(tuple(range(10))).st_atime == 7
 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()