diff --git a/Cargo.toml b/Cargo.toml index 0444095..5a75c01 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ const_format = { version = "0.2", features = ["const_generics"] } crossbeam-utils = "0.8" lazy_static = "1.4" log = { version = "0.4", features = ["max_level_trace", "release_max_level_info"] } +nix = { version = "0.22", default-features = false } regex = "1.5" serde = { version = "1.0", features = ["derive"]} shlex = "1.0.0" @@ -23,6 +24,3 @@ toml = "0.5" tree_magic_mini = "3.0" url = "2.2" xdg = "2.1" - -[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] -nix = { version = "0.22", default-features = false } diff --git a/src/handler.rs b/src/handler.rs index 97918d2..b0f025e 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -1,15 +1,11 @@ use std::collections::HashMap; use std::env; -#[cfg(any(target_os = "linux", target_os = "android"))] use std::fs::File; -use std::io::{self, stdin, Read, Write}; -#[cfg(not(any(target_os = "linux", target_os = "android")))] -use std::io::{copy, StdinLock}; +use std::io::{self, copy, Read, Write}; use std::os::unix::fs::FileTypeExt; -#[cfg(any(target_os = "linux", target_os = "android"))] use std::os::unix::io::{AsRawFd, FromRawFd}; use std::path::{Path, PathBuf}; -use std::process::{Child, ChildStdout, Command, Stdio}; +use std::process::{Child, Command, Stdio}; use std::rc::Rc; use crate::config; @@ -108,20 +104,6 @@ lazy_static::lazy_static! { nix::unistd::sysconf(nix::unistd::SysconfVar::PAGE_SIZE).expect("Unable to get page size").unwrap() as usize; } -#[cfg(not(any(target_os = "linux", target_os = "android")))] -trait ReadPipe: Read + Send {} - -#[cfg(any(target_os = "linux", target_os = "android"))] -trait ReadPipe: Read + AsRawFd + Send {} - -#[cfg(not(any(target_os = "linux", target_os = "android")))] -impl ReadPipe for StdinLock<'_> {} - -#[cfg(any(target_os = "linux", target_os = "android"))] -impl ReadPipe for File {} - -impl ReadPipe for ChildStdout {} - impl HandlerMapping { pub fn new(cfg: &config::Config) -> anyhow::Result { let mut handlers_open = FileHandlers::new(&cfg.default_handler_open); @@ -214,7 +196,11 @@ impl HandlerMapping { } pub fn handle_pipe(&self, mode: RsopMode) -> Result<(), HandlerError> { - let stdin = Self::stdin_reader()?; + let stdin_locked = io::stdin().lock(); + // Unlocked stdin via io::stdin does not allow use of copy optimisation (splice & co) + // The conversion to File via its fd allows breaking the Send requirement of the MutexGuard + let stdin = unsafe { File::from_raw_fd(stdin_locked.as_raw_fd()) }; + //let stdin = BufReader::new(stdin_file); self.dispatch_pipe(stdin, &mode) } @@ -301,7 +287,7 @@ impl HandlerMapping { #[allow(clippy::wildcard_in_or_patterns)] fn dispatch_pipe(&self, mut pipe: T, mode: &RsopMode) -> Result<(), HandlerError> where - T: ReadPipe, + T: Read + Send, { // Handler candidates let handlers = match mode { @@ -520,7 +506,7 @@ impl HandlerMapping { mode: &RsopMode, ) -> Result<(), HandlerError> where - T: ReadPipe, + T: Read + Send, { let term_size = Self::term_size(); @@ -609,7 +595,7 @@ impl HandlerMapping { term_size: &termsize::Size, ) -> Result<(), HandlerError> where - T: ReadPipe, + T: Read + Send, { // Write to a temporary file if handler does not support reading from stdin let input = if handler.no_pipe { @@ -683,26 +669,8 @@ impl HandlerMapping { Ok(()) } - #[cfg(not(any(target_os = "linux", target_os = "android")))] - fn stdin_reader() -> anyhow::Result> { - let stdin = Box::leak(Box::new(stdin())); - Ok(stdin.lock()) - } - - #[cfg(any(target_os = "linux", target_os = "android"))] - fn stdin_reader() -> anyhow::Result { - // Unfortunately, stdin is buffered, and there is no clean way to get it - // unbuffered to read only what we want for the header, so use fd hack to get an unbuffered reader - // see https://users.rust-lang.org/t/add-unbuffered-rawstdin-rawstdout/26013 - // On plaforms other than linux we don't care about buffering because we use chunk copy instead of splice - let stdin = stdin(); - let reader = unsafe { File::from_raw_fd(stdin.as_raw_fd()) }; - Ok(reader) - } - - #[cfg(not(any(target_os = "linux", target_os = "android")))] - // Default chunk copy using stdlib's std::io::copy when splice syscall is not available - fn pipe_forward(src: &mut S, dst: &mut D, header: &[u8]) -> anyhow::Result + // Default copy using stdlib's std::io::copy (uses splice syscall when available on Linux) + fn pipe_forward(src: &mut S, dst: &mut D, header: &[u8]) -> anyhow::Result where S: Read, D: Write, @@ -710,7 +678,7 @@ impl HandlerMapping { dst.write_all(header)?; log::trace!("Header written ({} bytes)", header.len()); - let copied = copy(src, dst)?; + let copied = copy(src, dst)? as usize; log::trace!( "Pipe exhausted, moved {} bytes total", header.len() + copied @@ -719,49 +687,9 @@ impl HandlerMapping { Ok(header.len() + copied) } - #[cfg(any(target_os = "linux", target_os = "android"))] - // Efficient 0-copy implementation using splice - fn pipe_forward(src: &mut S, dst: &mut D, header: &[u8]) -> anyhow::Result - where - S: AsRawFd, - D: AsRawFd + Write, - { - dst.write_all(header)?; - log::trace!("Header written ({} bytes)", header.len()); - - let mut c = 0; - const SPLICE_LEN: usize = 2usize.pow(62); // splice returns -EINVAL for pipe to file with usize::MAX len - const SPLICE_FLAGS: nix::fcntl::SpliceFFlags = nix::fcntl::SpliceFFlags::empty(); - - loop { - let rc = nix::fcntl::splice( - src.as_raw_fd(), - None, - dst.as_raw_fd(), - None, - SPLICE_LEN, - SPLICE_FLAGS, - ); - let moved = match rc { - Err(e) if e == nix::errno::Errno::EPIPE => 0, - Err(e) => return Err(anyhow::Error::new(e)), - Ok(m) => m, - }; - log::trace!("moved = {}", moved); - if moved == 0 { - break; - } - c += moved; - } - - log::trace!("Pipe exhausted, moved {} bytes total", header.len() + c); - - Ok(header.len() + c) - } - fn pipe_to_tmpfile(header: &[u8], mut pipe: T) -> anyhow::Result where - T: ReadPipe, + T: Read + Send, { let mut tmp_file = tempfile::Builder::new() .prefix(const_format::concatcp!(env!("CARGO_PKG_NAME"), '_')) diff --git a/test_splice b/test_splice new file mode 100755 index 0000000..f893908 --- /dev/null +++ b/test_splice @@ -0,0 +1,21 @@ +#!/bin/bash -eu + +set -o pipefail + + +# auto cleanup +at_exit() { + set +u + rm -Rf "$TMP_DIR" + set -u +} +trap at_exit EXIT + +readonly TMP_FILE="$(mktemp /tmp/"$(basename -- "$0")".XXXXXXXXXX)" + + +dd if=/dev/urandom of="${TMP_FILE}" bs=4k count=1000 + +cargo build --release + +RSOP_MODE=preview strace ./target/release/rsop <"${TMP_FILE}" 2>&1 | grep splice