diff --git a/Cargo.lock b/Cargo.lock index 3ecbdf9cea..d978a5d700 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1559,6 +1559,7 @@ dependencies = [ "cfg-if", "cfg_aliases", "libc", + "memoffset", ] [[package]] @@ -2458,7 +2459,7 @@ dependencies = [ "memchr", "memmap2", "mt19937", - "nix 0.29.0", + "nix 0.30.1", "num-complex", "num-integer", "num-traits", @@ -2533,7 +2534,7 @@ dependencies = [ "malachite-bigint", "memchr", "memoffset", - "nix 0.29.0", + "nix 0.30.1", "num-complex", "num-integer", "num-traits", diff --git a/Cargo.toml b/Cargo.toml index ddf45d65c4..bebcf94681 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -184,7 +184,7 @@ junction = "1.2.0" libc = "0.2.169" libffi = "4.1" log = "0.4.27" -nix = { version = "0.29", features = ["fs", "user", "process", "term", "time", "signal", "ioctl", "socket", "sched", "zerocopy", "dir", "hostname", "net", "poll"] } +nix = { version = "0.30", features = ["fs", "user", "process", "term", "time", "signal", "ioctl", "socket", "sched", "zerocopy", "dir", "hostname", "net", "poll"] } malachite-bigint = "0.6" malachite-q = "0.6" malachite-base = "0.6" diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 30d6b6d3c3..dc0c6fe51e 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -1993,7 +1993,9 @@ def test_open_file(self): with open(outfile, encoding="utf-8") as f: self.assertEqual(f.read(), 'hello') - # TODO: RUSTPYTHON: FileNotFoundError: [Errno 2] No such file or directory (os error 2): '@test_55144_tmp' -> 'None' + # TODO: RUSTPYTHON: the rust runtime reopens closed stdio fds at startup, + # so this test fails, even though POSIX_SPAWN_CLOSE does + # actually have an effect @unittest.expectedFailure def test_close_file(self): closefile = os_helper.TESTFN diff --git a/common/src/crt_fd.rs b/common/src/crt_fd.rs index b4c1ea3294..4fda2f3dcf 100644 --- a/common/src/crt_fd.rs +++ b/common/src/crt_fd.rs @@ -189,6 +189,10 @@ impl Owned { pub fn into_raw(self) -> Raw { self.inner.into_raw_fd() } + + pub fn leak<'fd>(self) -> Borrowed<'fd> { + unsafe { Borrowed::borrow_raw(self.into_raw()) } + } } #[cfg(unix)] diff --git a/stdlib/src/mmap.rs b/stdlib/src/mmap.rs index ed92b74d2f..143bdcb43d 100644 --- a/stdlib/src/mmap.rs +++ b/stdlib/src/mmap.rs @@ -27,11 +27,9 @@ mod mmap { use nix::sys::stat::fstat; use nix::unistd; use num_traits::Signed; - use std::fs::File; + use rustpython_common::crt_fd; use std::io::{self, Write}; use std::ops::{Deref, DerefMut}; - #[cfg(unix)] - use std::os::unix::io::{FromRawFd, RawFd}; fn advice_try_from_i32(vm: &VirtualMachine, i: i32) -> PyResult { Ok(match i { @@ -179,7 +177,7 @@ mod mmap { struct PyMmap { closed: AtomicCell, mmap: PyMutex>, - fd: RawFd, + fd: AtomicCell, offset: libc::off_t, size: AtomicCell, pos: AtomicCell, // relative to offset @@ -190,7 +188,7 @@ mod mmap { #[derive(FromArgs)] struct MmapNewArgs { #[pyarg(any)] - fileno: RawFd, + fileno: i32, #[pyarg(any)] length: isize, #[pyarg(any, default = MAP_SHARED)] @@ -340,7 +338,8 @@ mod mmap { } }; - if fd != -1 { + let fd = unsafe { crt_fd::Borrowed::try_borrow_raw(fd) }; + if let Ok(fd) = fd { let metadata = fstat(fd) .map_err(|err| io::Error::from_raw_os_error(err as i32).to_pyexception(vm))?; let file_len = metadata.st_size; @@ -366,19 +365,19 @@ mod mmap { let mmap_opt = mmap_opt.offset(offset.try_into().unwrap()).len(map_size); let (fd, mmap) = || -> std::io::Result<_> { - if fd == -1 { - let mmap = MmapObj::Write(mmap_opt.map_anon()?); - Ok((fd, mmap)) - } else { - let new_fd = unistd::dup(fd)?; + if let Ok(fd) = fd { + let new_fd: crt_fd::Owned = unistd::dup(fd)?.into(); let mmap = match access { AccessMode::Default | AccessMode::Write => { - MmapObj::Write(unsafe { mmap_opt.map_mut(fd) }?) + MmapObj::Write(unsafe { mmap_opt.map_mut(&new_fd) }?) } - AccessMode::Read => MmapObj::Read(unsafe { mmap_opt.map(fd) }?), - AccessMode::Copy => MmapObj::Write(unsafe { mmap_opt.map_copy(fd) }?), + AccessMode::Read => MmapObj::Read(unsafe { mmap_opt.map(&new_fd) }?), + AccessMode::Copy => MmapObj::Write(unsafe { mmap_opt.map_copy(&new_fd) }?), }; - Ok((new_fd, mmap)) + Ok((Some(new_fd), mmap)) + } else { + let mmap = MmapObj::Write(mmap_opt.map_anon()?); + Ok((None, mmap)) } }() .map_err(|e| e.to_pyexception(vm))?; @@ -386,7 +385,7 @@ mod mmap { let m_obj = Self { closed: AtomicCell::new(false), mmap: PyMutex::new(Some(mmap)), - fd, + fd: AtomicCell::new(fd.map_or(-1, |fd| fd.into_raw())), offset, size: AtomicCell::new(map_size), pos: AtomicCell::new(0), @@ -841,9 +840,8 @@ mod mmap { #[pymethod] fn size(&self, vm: &VirtualMachine) -> std::io::Result { - let new_fd = unistd::dup(self.fd)?; - let file = unsafe { File::from_raw_fd(new_fd) }; - let file_len = file.metadata()?.len(); + let fd = unsafe { crt_fd::Borrowed::try_borrow_raw(self.fd.load())? }; + let file_len = fstat(fd)?.st_size; Ok(PyInt::from(file_len).into_ref(&vm.ctx)) } diff --git a/stdlib/src/posixsubprocess.rs b/stdlib/src/posixsubprocess.rs index 744024e21f..7f418c8993 100644 --- a/stdlib/src/posixsubprocess.rs +++ b/stdlib/src/posixsubprocess.rs @@ -18,9 +18,8 @@ use std::{ io::prelude::*, marker::PhantomData, ops::Deref, - os::fd::FromRawFd, + os::fd::{AsFd, AsRawFd, BorrowedFd, IntoRawFd, OwnedFd, RawFd}, }; -use std::{fs::File, os::unix::io::AsRawFd}; use unistd::{Gid, Uid}; pub(crate) use _posixsubprocess::make_module; @@ -33,7 +32,7 @@ mod _posixsubprocess { use crate::vm::{PyResult, VirtualMachine, convert::IntoPyException}; #[pyfunction] - fn fork_exec(args: ForkExecArgs, vm: &VirtualMachine) -> PyResult { + fn fork_exec(args: ForkExecArgs<'_>, vm: &VirtualMachine) -> PyResult { if args.preexec_fn.is_some() { return Err(vm.new_not_implemented_error("preexec_fn not supported yet")); } @@ -59,7 +58,7 @@ mod _posixsubprocess { macro_rules! gen_args { ($($field:ident: $t:ty),*$(,)?) => { #[derive(FromArgs)] - struct ForkExecArgs { + struct ForkExecArgs<'fd> { $(#[pyarg(positional)] $field: $t,)* } }; @@ -121,21 +120,95 @@ impl CharPtrSlice<'_> { } } +#[derive(Copy, Clone)] +struct Fd(BorrowedFd<'static>); + +impl TryFromObject for Fd { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + match MaybeFd::try_from_object(vm, obj)? { + MaybeFd::Valid(fd) => Ok(fd), + MaybeFd::Invalid => Err(vm.new_value_error("invalid fd")), + } + } +} + +impl Write for Fd { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + Ok(unistd::write(self, buf)?) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +impl AsRawFd for Fd { + fn as_raw_fd(&self) -> RawFd { + self.0.as_raw_fd() + } +} + +impl IntoRawFd for Fd { + fn into_raw_fd(self) -> RawFd { + self.0.as_raw_fd() + } +} + +impl AsFd for Fd { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0.as_fd() + } +} + +impl From for Fd { + fn from(fd: OwnedFd) -> Self { + Self(unsafe { BorrowedFd::borrow_raw(fd.into_raw_fd()) }) + } +} + +#[derive(Copy, Clone)] +enum MaybeFd { + Valid(Fd), + Invalid, +} + +impl TryFromObject for MaybeFd { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + let fd = i32::try_from_object(vm, obj)?; + Ok(if fd == -1 { + MaybeFd::Invalid + } else { + MaybeFd::Valid(Fd(unsafe { BorrowedFd::borrow_raw(fd) })) + }) + } +} + +impl AsRawFd for MaybeFd { + fn as_raw_fd(&self) -> RawFd { + match self { + MaybeFd::Valid(fd) => fd.as_raw_fd(), + MaybeFd::Invalid => -1, + } + } +} + +// impl + gen_args! { args: ArgSequence /* list */, exec_list: ArgSequence /* list */, close_fds: bool, - fds_to_keep: ArgSequence, + fds_to_keep: ArgSequence>, cwd: Option, env_list: Option>, - p2cread: i32, - p2cwrite: i32, - c2pread: i32, - c2pwrite: i32, - errread: i32, - errwrite: i32, - errpipe_read: i32, - errpipe_write: i32, + p2cread: MaybeFd, + p2cwrite: MaybeFd, + c2pread: MaybeFd, + c2pwrite: MaybeFd, + errread: MaybeFd, + errwrite: MaybeFd, + errpipe_read: Fd, + errpipe_write: Fd, restore_signals: bool, call_setsid: bool, pgid_to_set: libc::pid_t, @@ -154,13 +227,12 @@ struct ProcArgs<'a> { extra_groups: Option<&'a [Gid]>, } -fn exec(args: &ForkExecArgs, procargs: ProcArgs<'_>) -> ! { +fn exec(args: &ForkExecArgs<'_>, procargs: ProcArgs<'_>) -> ! { let mut ctx = ExecErrorContext::NoExec; match exec_inner(args, procargs, &mut ctx) { Ok(x) => match x {}, Err(e) => { - let mut pipe = - std::mem::ManuallyDrop::new(unsafe { File::from_raw_fd(args.errpipe_write) }); + let mut pipe = args.errpipe_write; let _ = write!(pipe, "OSError:{}:{}", e as i32, ctx.as_msg()); std::process::exit(255) } @@ -184,49 +256,59 @@ impl ExecErrorContext { } fn exec_inner( - args: &ForkExecArgs, + args: &ForkExecArgs<'_>, procargs: ProcArgs<'_>, ctx: &mut ExecErrorContext, ) -> nix::Result { for &fd in args.fds_to_keep.as_slice() { - if fd != args.errpipe_write { - posix::raw_set_inheritable(fd, true)? + if fd.as_raw_fd() != args.errpipe_write.as_raw_fd() { + posix::set_inheritable(fd, true)? } } for &fd in &[args.p2cwrite, args.c2pread, args.errread] { - if fd != -1 { + if let MaybeFd::Valid(fd) = fd { unistd::close(fd)?; } } unistd::close(args.errpipe_read)?; - let c2pwrite = if args.c2pwrite == 0 { - let fd = unistd::dup(args.c2pwrite)?; - posix::raw_set_inheritable(fd, true)?; - fd - } else { - args.c2pwrite + let c2pwrite = match args.c2pwrite { + MaybeFd::Valid(c2pwrite) if c2pwrite.as_raw_fd() == 0 => { + let fd = unistd::dup(c2pwrite)?; + posix::set_inheritable(fd.as_fd(), true)?; + MaybeFd::Valid(fd.into()) + } + fd => fd, }; let mut errwrite = args.errwrite; - while errwrite == 0 || errwrite == 1 { - errwrite = unistd::dup(errwrite)?; - posix::raw_set_inheritable(errwrite, true)?; + loop { + match errwrite { + MaybeFd::Valid(fd) if fd.as_raw_fd() == 0 || fd.as_raw_fd() == 1 => { + let fd = unistd::dup(fd)?; + posix::set_inheritable(fd.as_fd(), true)?; + errwrite = MaybeFd::Valid(fd.into()); + } + _ => break, + } } - let dup_into_stdio = |fd, io_fd| { - if fd == io_fd { - posix::raw_set_inheritable(fd, true) - } else if fd != -1 { - unistd::dup2(fd, io_fd).map(drop) - } else { - Ok(()) + fn dup_into_stdio(fd: MaybeFd, io_fd: i32, dup2_stdio: F) -> nix::Result<()> + where + F: Fn(Fd) -> nix::Result<()>, + { + match fd { + MaybeFd::Valid(fd) if fd.as_raw_fd() == io_fd => { + posix::set_inheritable(fd.as_fd(), true) + } + MaybeFd::Valid(fd) => dup2_stdio(fd), + MaybeFd::Invalid => Ok(()), } - }; - dup_into_stdio(args.p2cread, 0)?; - dup_into_stdio(c2pwrite, 1)?; - dup_into_stdio(errwrite, 2)?; + } + dup_into_stdio(args.p2cread, 0, unistd::dup2_stdin)?; + dup_into_stdio(c2pwrite, 1, unistd::dup2_stdout)?; + dup_into_stdio(errwrite, 2, unistd::dup2_stderr)?; if let Some(ref cwd) = args.cwd { unistd::chdir(cwd.s.as_c_str()).inspect_err(|_| *ctx = ExecErrorContext::ChDir)? @@ -292,12 +374,16 @@ fn exec_inner( #[derive(Copy, Clone)] struct KeepFds<'a> { above: i32, - keep: &'a [i32], + keep: &'a [BorrowedFd<'a>], } impl KeepFds<'_> { fn should_keep(self, fd: i32) -> bool { - fd > self.above && self.keep.binary_search(&fd).is_err() + fd > self.above + && self + .keep + .binary_search_by_key(&fd, BorrowedFd::as_raw_fd) + .is_err() } } @@ -394,9 +480,13 @@ fn close_fds_brute_force(keep: KeepFds<'_>) { .ok() .flatten() .unwrap_or(256) as i32; - let fds = itertools::chain![Some(keep.above), keep.keep.iter().copied(), Some(max_fd)]; + let fds = itertools::chain![ + Some(keep.above), + keep.keep.iter().map(BorrowedFd::as_raw_fd), + Some(max_fd) + ]; for fd in fds.tuple_windows().flat_map(|(start, end)| start + 1..end) { - let _ = unistd::close(fd); + unsafe { libc::close(fd) }; } } diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs index fe6e20f08a..b1cccfafd7 100644 --- a/vm/src/stdlib/io.rs +++ b/vm/src/stdlib/io.rs @@ -3972,7 +3972,7 @@ mod _io { // check file descriptor validity #[cfg(unix)] if let Ok(crate::ospath::OsPathOrFd::Fd(fd)) = file.clone().try_into_value(vm) { - nix::fcntl::fcntl(fd.as_raw(), nix::fcntl::F_GETFD) + nix::fcntl::fcntl(fd, nix::fcntl::F_GETFD) .map_err(|_| crate::stdlib::os::errno_err(vm))?; } diff --git a/vm/src/stdlib/os.rs b/vm/src/stdlib/os.rs index 35c164d6d2..1c65d4d12c 100644 --- a/vm/src/stdlib/os.rs +++ b/vm/src/stdlib/os.rs @@ -378,8 +378,7 @@ pub(super) mod _os { #[cfg(all(unix, not(target_os = "redox")))] { use rustpython_common::os::ffi::OsStrExt; - let new_fd = - nix::unistd::dup(fno.as_raw()).map_err(|e| e.into_pyexception(vm))?; + let new_fd = nix::unistd::dup(fno).map_err(|e| e.into_pyexception(vm))?; let mut dir = nix::dir::Dir::from_fd(new_fd).map_err(|e| e.into_pyexception(vm))?; dir.iter() diff --git a/vm/src/stdlib/posix.rs b/vm/src/stdlib/posix.rs index 0f8251193d..5bafa1e758 100644 --- a/vm/src/stdlib/posix.rs +++ b/vm/src/stdlib/posix.rs @@ -1,9 +1,9 @@ // spell-checker:disable use crate::{PyRef, VirtualMachine, builtins::PyModule}; -use std::os::unix::io::RawFd; +use std::os::fd::BorrowedFd; -pub fn raw_set_inheritable(fd: RawFd, inheritable: bool) -> nix::Result<()> { +pub fn set_inheritable(fd: BorrowedFd<'_>, inheritable: bool) -> nix::Result<()> { use nix::fcntl; let flags = fcntl::FdFlag::from_bits_truncate(fcntl::fcntl(fd, fcntl::FcntlArg::F_GETFD)?); let mut new_flags = flags; @@ -43,7 +43,7 @@ pub mod module { env, ffi::{CStr, CString}, fs, io, - os::fd::{AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}, + os::fd::{AsFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd}, }; use strum_macros::{EnumIter, EnumString}; @@ -370,7 +370,7 @@ pub mod module { let dst = args.dst.into_cstring(vm)?; #[cfg(not(target_os = "redox"))] { - nix::unistd::symlinkat(&*src, args.dir_fd.raw_opt(), &*dst) + nix::unistd::symlinkat(&*src, args.dir_fd.get(), &*dst) .map_err(|err| err.into_pyexception(vm)) } #[cfg(target_os = "redox")] @@ -383,7 +383,7 @@ pub mod module { #[cfg(not(target_os = "redox"))] #[pyfunction] - fn fchdir(fd: RawFd, vm: &VirtualMachine) -> PyResult<()> { + fn fchdir(fd: BorrowedFd<'_>, vm: &VirtualMachine) -> PyResult<()> { nix::unistd::fchdir(fd).map_err(|err| err.into_pyexception(vm)) } @@ -432,12 +432,11 @@ pub mod module { nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW }; - let dir_fd = dir_fd.raw_opt(); match path { OsPathOrFd::Path(ref p) => { - nix::unistd::fchownat(dir_fd, p.path.as_os_str(), uid, gid, flag) + nix::unistd::fchownat(dir_fd.get(), p.path.as_os_str(), uid, gid, flag) } - OsPathOrFd::Fd(fd) => nix::unistd::fchown(fd.as_raw(), uid, gid), + OsPathOrFd::Fd(fd) => nix::unistd::fchown(fd, uid, gid), } .map_err(|err| { // Use `From for io::Error` when it is available @@ -855,7 +854,7 @@ pub mod module { } #[pyfunction] - fn get_inheritable(fd: RawFd, vm: &VirtualMachine) -> PyResult { + fn get_inheritable(fd: BorrowedFd<'_>, vm: &VirtualMachine) -> PyResult { let flags = fcntl::fcntl(fd, fcntl::FcntlArg::F_GETFD); match flags { Ok(ret) => Ok((ret & libc::FD_CLOEXEC) == 0), @@ -864,12 +863,12 @@ pub mod module { } #[pyfunction] - fn set_inheritable(fd: i32, inheritable: bool, vm: &VirtualMachine) -> PyResult<()> { - super::raw_set_inheritable(fd, inheritable).map_err(|err| err.into_pyexception(vm)) + fn set_inheritable(fd: BorrowedFd<'_>, inheritable: bool, vm: &VirtualMachine) -> PyResult<()> { + super::set_inheritable(fd, inheritable).map_err(|err| err.into_pyexception(vm)) } #[pyfunction] - fn get_blocking(fd: RawFd, vm: &VirtualMachine) -> PyResult { + fn get_blocking(fd: BorrowedFd<'_>, vm: &VirtualMachine) -> PyResult { let flags = fcntl::fcntl(fd, fcntl::FcntlArg::F_GETFL); match flags { Ok(ret) => Ok((ret & libc::O_NONBLOCK) == 0), @@ -878,7 +877,7 @@ pub mod module { } #[pyfunction] - fn set_blocking(fd: RawFd, blocking: bool, vm: &VirtualMachine) -> PyResult<()> { + fn set_blocking(fd: BorrowedFd<'_>, blocking: bool, vm: &VirtualMachine) -> PyResult<()> { let _set_flag = || { use nix::fcntl::{FcntlArg, OFlag, fcntl}; @@ -897,8 +896,8 @@ pub mod module { fn pipe(vm: &VirtualMachine) -> PyResult<(OwnedFd, OwnedFd)> { use nix::unistd::pipe; let (rfd, wfd) = pipe().map_err(|err| err.into_pyexception(vm))?; - set_inheritable(rfd.as_raw_fd(), false, vm)?; - set_inheritable(wfd.as_raw_fd(), false, vm)?; + set_inheritable(rfd.as_fd(), false, vm)?; + set_inheritable(wfd.as_fd(), false, vm)?; Ok((rfd, wfd)) } @@ -940,7 +939,7 @@ pub mod module { #[cfg(not(target_os = "redox"))] fn _fchmod(fd: BorrowedFd<'_>, mode: u32, vm: &VirtualMachine) -> PyResult<()> { nix::sys::stat::fchmod( - fd.as_raw_fd(), + fd, nix::sys::stat::Mode::from_bits(mode as libc::mode_t).unwrap(), ) .map_err(|err| err.into_pyexception(vm)) @@ -1223,8 +1222,7 @@ pub mod module { fn openpty(vm: &VirtualMachine) -> PyResult<(OwnedFd, OwnedFd)> { let r = nix::pty::openpty(None, None).map_err(|err| err.into_pyexception(vm))?; for fd in [&r.master, &r.slave] { - super::raw_set_inheritable(fd.as_raw_fd(), false) - .map_err(|e| e.into_pyexception(vm))?; + super::set_inheritable(fd.as_fd(), false).map_err(|e| e.into_pyexception(vm))?; } Ok((r.master, r.slave)) } @@ -1423,11 +1421,8 @@ pub mod module { .into_cstring(vm) .map_err(|_| vm.new_value_error("path should not have nul bytes"))?; - let mut file_actions = unsafe { - let mut fa = std::mem::MaybeUninit::uninit(); - assert!(libc::posix_spawn_file_actions_init(fa.as_mut_ptr()) == 0); - fa.assume_init() - }; + let mut file_actions = + nix::spawn::PosixSpawnFileActions::init().map_err(|e| e.into_pyexception(vm))?; if let Some(it) = self.file_actions { for action in it.iter(vm)? { let action = action?; @@ -1446,41 +1441,30 @@ pub mod module { "POSIX_SPAWN_OPEN path should not have nul bytes", ) })?; - unsafe { - libc::posix_spawn_file_actions_addopen( - &mut file_actions, - fd, - path.as_ptr(), - oflag, - mode, - ) - } + let oflag = nix::fcntl::OFlag::from_bits_retain(oflag); + let mode = nix::sys::stat::Mode::from_bits_retain(mode); + file_actions.add_open(fd, &*path, oflag, mode) } PosixSpawnFileActionIdentifier::Close => { let (fd,) = args.bind(vm)?; - unsafe { - libc::posix_spawn_file_actions_addclose(&mut file_actions, fd) - } + file_actions.add_close(fd) } PosixSpawnFileActionIdentifier::Dup2 => { let (fd, newfd) = args.bind(vm)?; - unsafe { - libc::posix_spawn_file_actions_adddup2(&mut file_actions, fd, newfd) - } + file_actions.add_dup2(fd, newfd) } }; - if ret != 0 { - let err = std::io::Error::from_raw_os_error(ret); + if let Err(err) = ret { + let err = err.into(); return Err(IOErrorBuilder::with_filename(&err, self.path, vm)); } } } - let mut attrp = unsafe { - let mut sa = std::mem::MaybeUninit::uninit(); - assert!(libc::posix_spawnattr_init(sa.as_mut_ptr()) == 0); - sa.assume_init() - }; + let mut attrp = + nix::spawn::PosixSpawnAttr::init().map_err(|e| e.into_pyexception(vm))?; + let mut flags = nix::spawn::PosixSpawnFlags::empty(); + if let Some(sigs) = self.setsigdef { use nix::sys::signal; let mut set = signal::SigSet::empty(); @@ -1491,37 +1475,39 @@ pub mod module { })?; set.add(sig); } - assert!( - unsafe { libc::posix_spawnattr_setsigdefault(&mut attrp, set.as_ref()) } == 0 - ); + attrp + .set_sigdefault(&set) + .map_err(|e| e.into_pyexception(vm))?; + flags.insert(nix::spawn::PosixSpawnFlags::POSIX_SPAWN_SETSIGDEF); } - // Handle new posix_spawn attributes - let mut flags = 0i32; - if let Some(pgid) = self.setpgroup { - let ret = unsafe { libc::posix_spawnattr_setpgroup(&mut attrp, pgid) }; - if ret != 0 { - return Err(vm.new_os_error(format!("posix_spawnattr_setpgroup failed: {ret}"))); - } - flags |= libc::POSIX_SPAWN_SETPGROUP; + attrp + .set_pgroup(nix::unistd::Pid::from_raw(pgid)) + .map_err(|e| e.into_pyexception(vm))?; + flags.insert(nix::spawn::PosixSpawnFlags::POSIX_SPAWN_SETPGROUP); } if self.resetids { - flags |= libc::POSIX_SPAWN_RESETIDS; + flags.insert(nix::spawn::PosixSpawnFlags::POSIX_SPAWN_RESETIDS); } if self.setsid { // Note: POSIX_SPAWN_SETSID may not be available on all platforms - #[cfg(target_os = "linux")] - { - flags |= 0x0080; // POSIX_SPAWN_SETSID value on Linux - } - #[cfg(not(target_os = "linux"))] - { - return Err(vm.new_not_implemented_error( - "setsid parameter is not supported on this platform", - )); + cfg_if::cfg_if! { + if #[cfg(any( + target_os = "linux", + target_os = "haiku", + target_os = "solaris", + target_os = "illumos", + target_os = "hurd", + ))] { + flags.insert(nix::spawn::PosixSpawnFlags::from_bits_retain(libc::POSIX_SPAWN_SETSID)); + } else { + return Err(vm.new_not_implemented_error( + "setsid parameter is not supported on this platform", + )); + } } } @@ -1535,13 +1521,10 @@ pub mod module { })?; set.add(sig); } - let ret = unsafe { libc::posix_spawnattr_setsigmask(&mut attrp, set.as_ref()) }; - if ret != 0 { - return Err( - vm.new_os_error(format!("posix_spawnattr_setsigmask failed: {ret}")) - ); - } - flags |= libc::POSIX_SPAWN_SETSIGMASK; + attrp + .set_sigmask(&set) + .map_err(|e| e.into_pyexception(vm))?; + flags.insert(nix::spawn::PosixSpawnFlags::POSIX_SPAWN_SETSIGMASK); } if let Some(_scheduler) = self.scheduler { @@ -1552,19 +1535,11 @@ pub mod module { ); } - if flags != 0 { - // Check for potential overflow when casting to c_short - if flags > libc::c_short::MAX as i32 { - return Err(vm.new_value_error("Too many flags set for posix_spawn")); - } - let ret = - unsafe { libc::posix_spawnattr_setflags(&mut attrp, flags as libc::c_short) }; - if ret != 0 { - return Err(vm.new_os_error(format!("posix_spawnattr_setflags failed: {ret}"))); - } + if !flags.is_empty() { + attrp.set_flags(flags).map_err(|e| e.into_pyexception(vm))?; } - let mut args: Vec = self + let args: Vec = self .args .iter(vm)? .map(|res| { @@ -1572,47 +1547,15 @@ pub mod module { .map_err(|_| vm.new_value_error("path should not have nul bytes")) }) .collect::>()?; - let argv: Vec<*mut libc::c_char> = args - .iter_mut() - .map(|s| s.as_ptr() as _) - .chain(std::iter::once(std::ptr::null_mut())) - .collect(); - let mut env = envp_from_dict(self.env, vm)?; - let envp: Vec<*mut libc::c_char> = env - .iter_mut() - .map(|s| s.as_ptr() as _) - .chain(std::iter::once(std::ptr::null_mut())) - .collect(); - - let mut pid = 0; - let ret = unsafe { - if spawnp { - libc::posix_spawnp( - &mut pid, - path.as_ptr(), - &file_actions, - &attrp, - argv.as_ptr(), - envp.as_ptr(), - ) - } else { - libc::posix_spawn( - &mut pid, - path.as_ptr(), - &file_actions, - &attrp, - argv.as_ptr(), - envp.as_ptr(), - ) - } - }; + let env = envp_from_dict(self.env, vm)?; - if ret == 0 { - Ok(pid) + let ret = if spawnp { + nix::spawn::posix_spawnp(&path, &file_actions, &attrp, &args, &env) } else { - let err = std::io::Error::from_raw_os_error(ret); - Err(IOErrorBuilder::with_filename(&err, self.path, vm)) - } + nix::spawn::posix_spawn(&*path, &file_actions, &attrp, &args, &env) + }; + ret.map(Into::into) + .map_err(|err| IOErrorBuilder::with_filename(&err.into(), self.path, vm)) } } @@ -1733,36 +1676,32 @@ pub mod module { } #[pyfunction] - fn dup(fd: i32, vm: &VirtualMachine) -> PyResult { + fn dup(fd: BorrowedFd<'_>, vm: &VirtualMachine) -> PyResult { let fd = nix::unistd::dup(fd).map_err(|e| e.into_pyexception(vm))?; - super::raw_set_inheritable(fd, false) + super::set_inheritable(fd.as_fd(), false) .map(|()| fd) - .map_err(|e| { - let _ = nix::unistd::close(fd); - e.into_pyexception(vm) - }) + .map_err(|e| e.into_pyexception(vm)) } #[derive(FromArgs)] - struct Dup2Args { + struct Dup2Args<'fd> { #[pyarg(positional)] - fd: i32, + fd: BorrowedFd<'fd>, #[pyarg(positional)] - fd2: i32, + fd2: OwnedFd, #[pyarg(any, default = true)] inheritable: bool, } #[pyfunction] - fn dup2(args: Dup2Args, vm: &VirtualMachine) -> PyResult { - let fd = nix::unistd::dup2(args.fd, args.fd2).map_err(|e| e.into_pyexception(vm))?; + fn dup2(args: Dup2Args<'_>, vm: &VirtualMachine) -> PyResult { + let mut fd2 = std::mem::ManuallyDrop::new(args.fd2); + nix::unistd::dup2(args.fd, &mut fd2).map_err(|e| e.into_pyexception(vm))?; + let fd2 = std::mem::ManuallyDrop::into_inner(fd2); if !args.inheritable { - super::raw_set_inheritable(fd, false).map_err(|e| { - let _ = nix::unistd::close(fd); - e.into_pyexception(vm) - })? + super::set_inheritable(fd2.as_fd(), false).map_err(|e| e.into_pyexception(vm))? } - Ok(fd) + Ok(fd2) } pub(crate) fn support_funcs() -> Vec { diff --git a/vm/src/stdlib/signal.rs b/vm/src/stdlib/signal.rs index ff59208ade..bab45b3415 100644 --- a/vm/src/stdlib/signal.rs +++ b/vm/src/stdlib/signal.rs @@ -270,13 +270,16 @@ pub(crate) mod _signal { false }; #[cfg(unix)] - if fd != INVALID_WAKEUP { + if let Ok(fd) = unsafe { crate::common::crt_fd::Borrowed::try_borrow_raw(fd) } { use nix::fcntl; let oflags = fcntl::fcntl(fd, fcntl::F_GETFL).map_err(|e| e.into_pyexception(vm))?; let nonblock = fcntl::OFlag::from_bits_truncate(oflags).contains(fcntl::OFlag::O_NONBLOCK); if !nonblock { - return Err(vm.new_value_error(format!("the fd {fd} must be in non-blocking mode"))); + return Err(vm.new_value_error(format!( + "the fd {} must be in non-blocking mode", + fd.as_raw() + ))); } }